Changeset updates #2324
Changeset updates #2324
Conversation
- Add release/snapit documentation - Remove js shipit stacks - Update snapshot tag to be 'snapshot' insead of 'snapshot-release' - Go back to using the default GITHUB_TOKEN as a personal access token is not worth it
.github/PULL_REQUEST_TEMPLATE.md
Outdated
| @@ -3,23 +3,5 @@ | |||
| Fixes (issue #) | |||
|
|
|||
| <!-- | |||
| Please include a summary of what you want to achieve in this pull request. Remember to indicate the affected package(s). | |||
| Please include a summary of what you want to achieve in this pull request. Remember to add a changeset that indicates the affected package(s) and if they are major / minor / patch changes by using `yarn changeset`. | |||
There was a problem hiding this comment.
add a changeset
I'd suggest link-ifying this section of text and have it point to the section in our release guidelines that talks about changesets. It'll be new tech to a bunch of folks, and knowing how developers can get nerd-sniped I'd expect that a bunch will bail out of the sentence at this point to go read about what changesets are. Linking them will help us control where they find that info instead of starting with the public docs on changesets.
There was a problem hiding this comment.
Added a link to https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md, which is what the Changeset Bot does, e.g. changesets/changesets#844 (comment)
| @@ -1,11 +0,0 @@ | |||
| ci: | |||
Co-authored-by: Ryan Wilson-Perkin <ryanwilsonperkin@gmail.com>
|
Turns out explicitly setting the GITHUB_TOKEN was important - 6a91db7 |
Description
Changeset docs and tidying
is not worth it. Educating releasers that absent CI checks on version PRs is expected is easier than having to manage PAT tokens.
Whats all this about skipping CI checks and PATs?
As noted in the release doc, pull requests / commits authenticated with the default automatically provided GITHUB_TOKEN secret do not trigger downstream actions. The "Version Packages" PR and the commits therein are created with this token, which means there our CI actions do not get triggered. These CI actions are marked as required, so you end up with these checks being marked as required but never sent:
That's not ideal. I've tried to come up with a good way around this, and everything is worse than saying "yeah we know, use repo administrator privileges to force the merge even though those required checks are missing". Oh well.
Alternative "Fix" 1: Use a Personal Access Token (PAT)
By uploading a Personal Access Token and using its credentials when pushing commits the it should be possible to cause the pull_request events to trigger, and thus make the CI checks run and be associated with the commits.
However this means that now I have to manually cycle an access token whenever it expires. Having a long expiry is a bad idea as it means that if the token gets compromised then somebody can act as me without my knowledge. I'd have to do this in every repo we want to use changesets in (quilt, web-configs, sewing-kit, loom) and that overhead multiplies quickly. I don't want to be a single point of failure, and having to teach folks "every few months you need to update this token across many repos" is a terrible experience.
Alternative "Fix 2: Use some common token
Riffing on the PAT idea, what if there was an organisation level secret that we could use? That'd be great but it does not exist. I've spoke to code-scale about this - they're understandably against using service accounts (having been moving away from that idea for years). There was some ideas about them building some token rotator that could update something in an org-level secret that many repos could have access too (kinda like the NPM_TOKEN), but that'll require a lot of working. It's a nice idea for the future but it isn't ready now.
Alternative "Fix" 3: Trigger the CI actions manually
I thought it might be possible to trigger the CI actions manually. I couldn't get it work. I suspect that there's a difference between "this workflow is ran" and "this adds a check against a specific commit". I could do the former but not the latter.
Alternative "Fix" 4: Remove those checks from being "required checks"
Uugh, no. I don't want to remove the notion of "these are required" on PRs made by humans. These being required makes sure that PRs can't be merged in the cases where webhooks are delayed and it takes a while for the CI actions are triggered. I could not see a way of marking "commits by this person can bypass the checks requirement".
Current state of the world
I think telling people "yes we know these checks shall be absent on the Version PR, but we think that is ok and you should use your admin-ness to merge this PR" is better than "having personal access tokens across multiple repositories" or "losing 'required' marks on PRs made by humans".