Add release action

[palewire]

Here you find a basic implementation of a release action. When a Github Release is triggered, it will run after the successful completion of the of the lint and test steps. In my testing, it is successfully uploading an empty package to my personal npmjs.com account.

However, it may need to be customized to support the yarn build rollup step in this repository. Rather than try to guess how you'd like that part to work, I thought I'd put this up here as a starting point for discussion. Let me know what more I can do to improve the proposal.

[hobbes7878]

Cool. Just outlining the release job to make sure I get it: We add a git tag for new patch/minor/major on master. That then bootstraps and builds (missing rn, as you said) the library. Finally, npm versions and publishes to the registry with the tag we pushed (so need to consistently tag as 1.0.1 not v1.0.0, I think??).

Other Qs: Do we need to restrict this job to just master branch and just our repo (e.g.)? Do we need to commit back to the repo, since npm-version is updating package.json with the new version number? Would it make sense to throw release in its own yaml restricted to push on master?

[palewire]

Cool. Just outlining the release job to make sure I get it: We add a git tag for new patch/minor/major on master. That then bootstraps and builds (missing rn, as you said) the library. Finally, npm versions and publishes to the registry with the tag we pushed (so need to consistently tag as 1.0.1 not v1.0.0, I think??).

Yes, that's basically it. Though it should stamp the version with whatever you pass into the release name in GitHub's web interface.

Other Qs: Do we need to restrict this job to just master branch and just our repo (e.g.)?

The release event that qualifies to run the new, final step of the action should only get triggered for the default branch, I think. We want the tests that precede it to run for every push to every branch, so I wouldn't want to add a workflow-level branch restriction. I could imagine adding another statement to the if clause if you can imagine a circumstance where a tagged release would run for another branch.

The alternative method for handling the event, which is recommended by the GitHub docs, is to craft a separate release action that runs on the published hook that fires after a GitHub Release is created. I tend not to use that approach because I like having the linter and test jobs act as hurdles to prevent broken code from shipping, and I prefer not to replicate those jobs in two different actions. That said, this alternative structure totally works and I'd be happy to switch to using it here.

Do we need to commit back to the repo, since npm-version is updating package.json with the new version number? Would it make sense to throw release in its own yaml restricted to push on master?

You've put your finger on a potential problem in the code here, I think. Currently it is not automatically updating the version attribute in package.json to match the version number that's been shipped to the registry. There must be a way to make this work but I haven't put the time in to sort it out. Right now that would have to be updated manually prior to sending the release out with the same version number. We could make that extra feature a requirement before we merge, if you like.

[hobbes7878]

Yes, that's basically it. Though it should stamp the version with whatever you pass into the release name in GitHub's web interface.

Ah, that's right. I forgot about the web release interface. Was thinking of this triggering on, e.g., git tag 1.0.0 (which I think it will??).

The release event that qualifies to run the new, final step of the action should only get triggered for the default branch, I think. We want the tests that precede it to run for every push to every branch, so I wouldn't want to add a workflow-level branch restriction. {{ Yep, agree. }} I could imagine adding another statement to the if clause if you can imagine a circumstance where a tagged release would run for another branch.

So I see the startsWith(github.ref, 'refs/tags') in the new release job stops it from running on anything other than a tag. But if somebody created a PR with a tag on a branch, would that get through here?

I tend not to use that approach because I like having the linter and test jobs act as hurdles to prevent broken code from shipping, and I prefer not to replicate those jobs in two different actions.

👍

There must be a way to make this work but I haven't put the time in to sort it out. Right now that would have to be updated manually prior to sending the release out with the same version number. We could make that extra feature a requirement before we merge, if you like.

Hmm.. I think this might be easy enough to just commit back to the repo. npm version is gonna bump package.json automatically, so we just push that 1-line change back, and because action pushes don't trigger subsequent actions, I think we're in the clear??

Happy to help write this up. My only blocker is the first Q there about manual tags added on branches.

[palewire]

1. I don't know what will happen if you do a manual tag with git. My habit has been to limit my usage to the Releases interface here on the web. I can test it on my demo repo and see what happens.
2. Committing back to the repo should be doable, in the same manner as a scraper. Though I think that the tagged action checks out a detached head and it might require some kind of slick git maneuver to commit back to the default branch.

I can put these two items on my homework list and get back to you.

[palewire]

Your suspicion was correct. Manually creating a tag and pushing it a branch did slip past the if clause and trigger the release job. That's bad. I've pushed a patch. Here's are the receipts on the bug, which should now be resolved.

I'm still working on the second bit of homework.

Before

# palewire @ archbook in ~/Code/npm-ci-test on git:main o [4:27:58] 
$ git status
On branch main
Your branch is up to date with 'origin/main'.

nothing to commit, working tree clean

# palewire @ archbook in ~/Code/npm-ci-test on git:main o [4:27:59] 
$ git checkout -b test-tag
Switched to a new branch 'test-tag'

# palewire @ archbook in ~/Code/npm-ci-test on git:test-tag o [4:28:08] 
$ git tag 1.0.0

# palewire @ archbook in ~/Code/npm-ci-test on git:test-tag o [4:28:19] 
$ git push origin test-tag --tags
Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
To https://github.com/palewire/npm-ci-test.git
 * [new branch]      test-tag -> test-tag
 * [new tag]         1.0.0 -> 1.0.0

After

# palewire @ archbook in ~/Code/npm-ci-test on git:vet-tags o [5:04:01] C:128
$ git checkout -b vet-the-branch
Switched to a new branch 'vet-the-branch'

# palewire @ archbook in ~/Code/npm-ci-test on git:vet-the-branch o [5:04:14] 
$ git tag 0.0.999

# palewire @ archbook in ~/Code/npm-ci-test on git:vet-the-branch o [5:04:22] 
$ git push origin vet-the-branch --tags
Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
To https://github.com/palewire/npm-ci-test.git
 * [new branch]      vet-the-branch -> vet-the-branch
 * [new tag]         0.0.999 -> 0.0.999

[palewire]

I believe I've handled the commit issue. The key trick was adding the contents permission to the job. This pull request is again ready for your review.

[palewire]

I've added documentation explaining the release process this enables.

[palewire]

Hey, if it works it's better. My only nit pick would be that a docstring or two might be nice in something that long.

[hobbes7878]

K. Tried out a trim of your OG that simplifies a few bits and I think will apply well across most of our other node libs, too.

Here's the idea: https://github.com/reuters-graphics/npm-ci-test/blob/main/.github/workflows/ci.yaml

Checks I've done:

• [x] Runs tests on all pushes/PRs
• [x] Runs release only on tags
• [x] Does not run release on branch tags
• [x] Does not run release on tags on forks

Open question from me, I'm not sure when we'd run this on a workflow_dispatch. I'm guessing for integrating with other CI tools, but there's no real harm in't, I suppose.

If we're happy with something like above, happy to commit to your PR on my end.

[palewire]

Okay. I've tried to reconcile my patch to use your new if statement. I'm agnostic on the workflow_dispatch, though I do like it to run an ad hoc test run every once in a while. Happy to lose it if that's what you'd prefer.

Small bugs asides, my only major question left is whether the packaging commands I've included will work with this package. Are you doing any rollup work or other building locally before you upload a distribution with npm publish? If so, that will likely need to be added yet.

[hobbes7878]

I shoulda said, normally we run build in a prepublishOnly lifecycle script, so it's covered when we run npm publish.

In this repo, that's also tied into running tests, so yeah, probably a bit of cleanup to dedup for this new pattern.

[palewire]

Okay. Am I right to take it that would mean reduce prePublishOnly to npm run build? Are you including any additional flags to npm publish that instruct it to upload the dist folder created by build?

[hobbes7878]

Am I right to take it that would mean reduce prePublishOnly to npm run build?

yep!

any additional flags to npm publish that instruct it to upload the dist folder created by build?

The files prop in package.json handles that one.

[palewire]

Thank you. I believe I've made that edit to the package.json's prePublishOnly command.