Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Canary version not being added to some PRs in GitHub Action #481

Closed
zephraph opened this issue Jul 9, 2019 · 21 comments · Fixed by #812
Closed

Canary version not being added to some PRs in GitHub Action #481

zephraph opened this issue Jul 9, 2019 · 21 comments · Fixed by #812
Labels
bug Something isn't working released This issue/pull request has been released.

Comments

@zephraph
Copy link
Collaborator

zephraph commented Jul 9, 2019

Describe the bug

I've noticed that on the PRs I create that even though the canary versions are published the PR body isn't updated to have the version number.

To Reproduce

See any of my PRs for a repo.

Expected behavior

I created a repo, a canary version was deployed, but the canary PR edit wasn't made to the bottom of the repo.

Additional context

I assume that this is actually a permissions issue.

@zephraph zephraph added the bug Something isn't working label Jul 9, 2019
@hipstersmoothie
Copy link
Collaborator

@zephraph Have you seen this issue pop up lately?

@zephraph
Copy link
Collaborator Author

I have not. Will close.

@hipstersmoothie
Copy link
Collaborator

This could very well have happened like this:

  1. push code
  2. take a long time writing pr message
  3. canary version publishes before you submit PR and tries to post comment to PR that hasn't yet opened
  4. no canary version posted to PR

@hipstersmoothie
Copy link
Collaborator

@zephraph This might be an opportunity for autobot to add the canary version.

@glambert
Copy link

I see this also on 8.0.0, turning on the logs shows:

✔  success   Published canary version: <details><summary>Canary Versions</summary>- `[email protected]`
- `[email protected]`</details>

But the PR comment is not updated. I'm using the default auto init setup with npm and released plugins.

@hipstersmoothie
Copy link
Collaborator

Was the PR open at the time that the canary was published?

If you do the following the canary version will not post:

  1. push code to new branch
  2. ci runs build
  3. sometimes after build you open PR
  4. version never posted back

@glambert
Copy link

glambert commented Dec 14, 2019

Build finishes after I have created the PR.

If it did finish before the PR was created, when I push new commits it still doesn’t update. Does it need to work the first time so that extra commits also update the PR description?

Thanks again for the support

@hipstersmoothie
Copy link
Collaborator

Does it need to work the first time so that extra commits also update the PR description?

It should not matter for subsequent builds. Any chance I can check out the repo?

Maybe your PR isn't being detected on your build platform. I'll add some logs here

https://github.com/intuit/auto/blob/master/packages/core/src/auto.ts#L692

@hipstersmoothie
Copy link
Collaborator

Looking at it more most everything should already be in the logs. Can you turn on veryVerbose and post a log?

We should be able to see:

  1. the PR number detected https://github.com/intuit/auto/blob/master/packages/core/src/auto.ts#L653
  2. PR body will also log all of what it does https://github.com/intuit/auto/blob/master/packages/core/src/auto.ts#L555

@glambert
Copy link

glambert commented Dec 15, 2019

So PR is indeed not found for Canary release (although it gets the PR number in the first log line):

info      Getting commits for PR #4
302
ℹ  info      {
303
  currentBranch: 'package-2-update',
304
  isBaseBrach: false,
305
  isPR: false,
306
  shouldGraduate: true,
307
  isPrereleaseBranch: false,
308
  publishPrerelease: false
309
}
310
ℹ  info      Canary info found: { pr: undefined, build: undefined }
311
ℹ  info      Getting commits from HEAD^ to HEAD

FYI I’m using lerna in independent mode. The repo is internal but its a POC, so I’ll probably move it to a public one so I can show you, it’ll be easier to debug this way 🙂 Monday tho 😉

@glambert
Copy link

glambert commented Dec 16, 2019

@hipstersmoothie
Copy link
Collaborator

hipstersmoothie commented Dec 16, 2019 via email

@hipstersmoothie
Copy link
Collaborator

Ah this may be tied to some caveats I've found with GitHub actions.

This library isn't detecting the branch/PR correctly semantic-release/env-ci#96

The Getting commits for PR #4 comes from us matching the commit SHA to a PR, not the env info unfortunately.

I'm gonna play around with it a little to see if I can get env-ci to behave like I want it to

@hipstersmoothie hipstersmoothie changed the title Canary version not being added to some PRs Canary version not being added to some PRs in GitHub ACtion Dec 16, 2019
@hipstersmoothie hipstersmoothie changed the title Canary version not being added to some PRs in GitHub ACtion Canary version not being added to some PRs in GitHub Action Dec 16, 2019
@hipstersmoothie
Copy link
Collaborator

It seems like the push event might not be what we want?

https://mobile.twitter.com/ethomson/status/1176421835157704704

@hipstersmoothie
Copy link
Collaborator

I have an incoming PR that will find the PR that the head commit is associated with for canary versions. This should help with finding the PR to comment on

@glambert
Copy link

glambert commented Dec 16, 2019

Ahhh right, so you’d have both push and pull_request as triggers?

I’ll give a try with the canary version of the PR and let you know if it works 👍 in the end I might not use canary releases with lerna independant mode because it would create canary for all packages even unchanged ones. But at least I’ll know it works 😉

@hipstersmoothie
Copy link
Collaborator

Ahhh right, so you’d have both push and pull_request as triggers?

I haven't used GitHub actions all that much so IDK what the best workflow is quite yet (hopefully you can help us find it 😃). To me having multiple actions set up is kinda too much overhead. If my canary works this should alleviate the issue for the most part.

in the end I might not use canary releases with lerna independant mode because it would create canary for all packages even unchanged ones

Since multiple users have now requested that I don't force publish canaries I am open to it. But in my experience I have found that you want to force publish them for edge cases.

For instance if you change your babel config and want to test the changes on a canary version. Without force publishing lerna will not publish all anything if you just change a global config file, since no packages have actually changed.

What are your reasons for not wanting everything to update in a canary? I am not an independant user so there might be something I am missing.

@glambert
Copy link

What are your reasons for not wanting everything to update in a canary? I am not an independant user so there might be something I am missing.

We have a repository of like 20+ libs (and growing) so doing canary releases for all of these on each PR will get very noisy. What I was thinking is instead if perhaps use the next branch strategy. The good thing about using canary tho is you can test out individual commits. Is there a way to trigger canary "manually" for specific packages in independent mode perhaps? For example: run lerna changed and create canary releases for only those packages who changed. Thinking out loud here 😉

@hipstersmoothie
Copy link
Collaborator

not currently.

https://github.com/intuit/auto/blob/master/plugins/npm/src/index.ts#L562

I am open to adding a flag/config like --no-force-publish-canary. I do want to keep the default as it is.

@glambert
Copy link

I am open to adding a flag/config like --no-force-publish-canary. I do want to keep the default as it is

Agreed, for most cases this is what you would want.

@adierkens
Copy link
Collaborator

🚀 Issue was released in v9.1.0 🚀

@adierkens adierkens added the released This issue/pull request has been released. label Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants