-
Notifications
You must be signed in to change notification settings - Fork 43
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: major version tags #682
Conversation
This consolidates getNodeVersion into another file and renames the set-version-output function to have a .js extension so we don't need to have weird `npm run lint` carveouts for it
This pattern for setting outputs is now deprecated
we were previously using a zero-deps approach but now that we need to install deps for the set-version-output step, we might as well use a safer approach here.
this will set up some upcoming tagging changes
This reverts commit 0fc0fe2.
simple-git isn't really being super helpful here, so we might as well use a vanilla node function instead. i struggled to write tests so not gonna use our `getPkgVersion` function either since importing directly from package.json is just fine.
i feel like this flow is better, tagging a beta tripped me up the first time
we should surface the v8 tag in the github actions that we create. this required a decent refactor to consolidate our major version retrieval logic. a bunch of snapshots had to be updated too.
core.setOutput('RDME_VERSION', `v${await getMajorPkgVersion('latest')}`); | ||
core.setOutput('NODE_VERSION', getNodeVersion()); |
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.
The diff got lost in renaming bin/set-version-output
to bin/set-version-output.js
(which is used for updating the package version in our docs with every sync to main
). Two major callouts:
- We're now using
core.setOutput
due to a deprecation notice with the::set-output
technique for setting Actions outputs. It's much cleaner too ✨ - We're setting the version in our docs to the major version now (e.g.,
v8
instead of8.1.1
).
@@ -65,4 +65,4 @@ In the example above, every push to the `main` branch will sync the Markdown con | |||
|
|||
> 📘 Keeping `rdme` up-to-date | |||
> | |||
> Note that `@RDME_VERSION` (used in the above example) is the latest version of `rdme`. We recommend [configuring Dependabot to keep your actions up-to-date](https://docs.github.com/code-security/supply-chain-security/keeping-your-dependencies-updated-automatically/keeping-your-actions-up-to-date-with-dependabot). | |||
> Note that `@RDME_VERSION` (used in the above example) is the latest version of `rdme`. We recommend [configuring Dependabot to keep your actions up-to-date](https://docs.github.com/code-security/dependabot/working-with-dependabot/keeping-your-actions-up-to-date-with-dependabot). |
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.
Noticed these links were redirecting, so just fixed them up throughout our docs.
@@ -97,15 +97,16 @@ | |||
"build": "tsc", | |||
"bump": "npm version -m 'build: %s release'", | |||
"debug": "ts-node src/cli.ts", | |||
"lint": "eslint . bin/rdme bin/set-version-output --ext .js,.ts", |
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.
By using the .js
file extensions in those bin files, we can omit them from here and they still work as command-line executables. I'm keeping rdme
here for now since it's special 💙
* | ||
* @example 14 | ||
*/ | ||
module.exports = function getNodeVersion() { |
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.
Moved this function over to src/lib/getPkgVersion.ts
1. We've been keeping this vanilla and as a JS file so we didn't have to install deps prior to syncing our docs, but installing deps has been required for a bit now, so might as well migrate and consolidate.
Footnotes
-
its test coverage was also moved to
__tests__/lib/getPkgVersion.test.ts
↩
src/lib/getPkgVersion.ts
Outdated
if (npmDistTag) { | ||
return fetch(registryUrl) | ||
.then(res => res.json()) | ||
.then(body => body['dist-tags'][npmDistTag]) | ||
.catch(() => { | ||
.catch(err => { | ||
// eslint-disable-next-line no-console | ||
console.error('error fetching version from npm registry'); | ||
console.error('error fetching version from npm registry:', err.message); | ||
return pkg.version; | ||
}); |
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 also added some test coverage for this in __tests__/lib/getPkgVersion.test.ts
.
const parsedVersion = semverParse(pkg.version); | ||
|
||
if (parsedVersion.prerelease.length) { | ||
// eslint-disable-next-line no-console | ||
console.warn('Pre-release version, not setting major version tag'); | ||
return; | ||
} |
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.
We could use our getPkgVersion
and getMajorPkgVersion
functions here, but I wanted to keep this fairly vanilla so we can refer to it for a hypothetical future blog post. Plus we need some semver logic here that is pretty specific.
🧰 Changes
I've always really liked how
actions/setup-node
andactions/checkout
use top-level tags for major versions, which are recreated every time there's a tag for that major version. It makes it a lot easier to point to the latest in a workflow (as well as in the docs):We've been shipping a lot of updates lately (a good thing!), but it's gotta be annoying for people that have Dependabot set up, since they're likely using an outdated version already. Now that we have a streamlined release process via
npm run bump
, I figured we should enhance the process a bit so it automatically recreates a major tag with every release.As part of this:
🧬 QA & Testing
I've done a few dry runs of
npm run bump
and confirmed that the tag is created:I also tried with a prerelease tag and confirmed that the major tag is not created:
You can follow these steps to try it out yourself:
npm run bump -- 8.1.2
ornpm run bump -- 8.1.2-beta.0
git tag -d [tag]
) and don't push anything to GitHub.(fixes RM-4942)