-
Notifications
You must be signed in to change notification settings - Fork 22
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
feat(ci): Automate Docker image publication when merging on master #1142
Conversation
I removed CircleCI configuration in this PR, but CircleCI checks are still running because the configuration is still present on We have another mention of CircleCI in // reporter for circleci
reporters: [
'default',
[
'jest-junit',
{
outputDirectory: 'junit',
suiteNameTemplate: '{filepath}',
ancestorSeparator: ' › ',
addFileAttribute: 'true',
},
],
], https://github.com/algolia/npm-search/blob/master/jest.config.js#L20-L31 |
^ that part of the jest config is so that circleCI shows which tests pass, which ones fail etc. GitHub Actions doesn't have that feature |
scripts/publish-check.js
Outdated
}, | ||
// Redirect output to new streams, to make the script silent | ||
{ | ||
stdout: new WritableStreamBuffer(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as we're not using them, I wonder if a builtin writable stream works too? no big deal though, but could avoid the dependency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar enough with streams to have found a builtin writable one. I went with the same dependency as the one actually used in semantic-release
. Any pointer on what I could try to use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be done with:
import {Writable} from 'node:stream'
new Writable({write(_chunk, _encoding, callback) { setImmediate(callback) }})
(but obviously this dep is also fine)
"docker:build": "./scripts/build.sh", | ||
"docker:release": "VERSION=$(node -e \"console.log(require('./package.json').version)\") && echo \"Releasing $VERSION\" && docker push algolia/npm-search && docker push algolia/npm-search:$VERSION" | ||
"publish:check": "node ./scripts/publish-check.js", | ||
"publish:github": "./scripts/publish-github", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could just be:
"publish:github": "./scripts/publish-github", | |
"publish:github": "semantic-release", |
and the file removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. I initially had the external script because there was a bit more code before and after, but you're right that it can now be simplified
@Haroenv I removed the stream dependency and the CircleCI-specific jest configuration. I kept the external script for |
FYI, the build on master failed with I assume this is an artefact of me removing CircleCI configuration on that PR, but it still technically being there when attempting to merge |
I updated the repo settings about branch protection. The previous CircleCI build job was explicitly set as a mandatory condition for the protected branch. Commits had to come from a branch where this check was green (which wasn't the case on this PR as I had disabled the whole of CircleCI). I moved it to the new GitHub Actions Validate job. I might need to do another debug PR to trigger it again |
This PR adds a second workflow,
publish
, to the existing GitHub Actions workflow.This one will be triggered on every commit on
master
(so, on every PR merge, asmaster
is protected). It will then:validate
workflow again, just to be sure we don't have any Lint/Build/Test issuesBREAKING CHANGE
/feat()
orfix()
package.json
(major, minor or patch, depending on the type of commit), and updateCHANGELOG.md
with the list of changes.The bulk of the job is done by the
semantic-release
package, but I had to add a few tweaks:I made the previous
validate
workflow re-usable, to avoid code duplication. There is still some code duplication to checkout, install node and its dependencies, but I don't think we can shave it more.I added a
yarn publish:check
script that simply checks if a new version should be released (by checking the commit history). The exit code of that script is then used in the workflow to conditionally continue the publication or not, but still keep the workflow as a success even if there is no publish to do.I cleaned up the
release.config.js
file to make our intent more explicit. It was previously relying on a lot of default values and only overriding the ones we weren't interested in. I reversed it and explictly define the one we want. Hopefully the workflow will be easier to understand.semantic-release
doesn't have a way to do GitHub Packages publications, so I created theyarn publish:docker
for that. It builds the image (with the requires tags and labels), login to ghcr.io (thankfully, GitHub Actions provides a one-time-useGITHUB_TOKEN
with all the required permission on each run), and push the new image (with the specific version and overriding thelatest
tag)Note: I specifically named the PR commit with
feat()
so once merged on master it should trigger a new release. I did test the workflow as much as I could on my branch in dry run mode, but the real test will be once this PR gets merged.