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

Update documentation for GitHub Actions Setup #2140

Closed
privatenumber opened this issue Sep 20, 2021 · 12 comments · Fixed by #2175 · May be fixed by #2176
Closed

Update documentation for GitHub Actions Setup #2140

privatenumber opened this issue Sep 20, 2021 · 12 comments · Fixed by #2175 · May be fixed by #2176

Comments

@privatenumber
Copy link

privatenumber commented Sep 20, 2021

New feature motivation

With the release of semantic-release v18, I imagine a lot of CI releases would be breaking due to a condition that requires a version of Node.js that was only released 4 months ago.

It broke mine:

[semantic-release]: node version >=14.17 is required. Found v12.18.2.

See semantic-release/semantic-release@master/docs/support/node-version.md for more details and solutions.

New feature description

  • To recommend pinning npx semantic-release to the current major in examples (eg. Github Actions setup should say npx 'semantic-release@^18.0.0').
  • For the v18.0.0 release notes to have more descriptive changes. I can see the specific changes in the beta release logs, but for those that are wondering why such a new version of Node.js is necessary, it's not very discoverable.
@gr2m
Copy link
Member

gr2m commented Sep 22, 2021

* To recommend pinning `npx semantic-release` to the current major in examples (eg. [Github Actions setup](https://github.com/semantic-release/semantic-release/blob/1405b94296059c0c6878fb8b626e2c5da9317632/docs/recipes/github-actions.md#githubworkflowsreleaseyml-configuration-for-node-projects) should say `npx 'semantic-release@^18.0.0'`).

an update to the docs would be much appreciated. We should replace actions/checkout@v1 with actions/checkout@v2, actions/setup-node@v1 with actions/setup-node@v2 and node-version with lts/*

  • For the v18.0.0 release notes to have more descriptive changes. I can see the specific changes in the beta release logs, but for those that are wondering why such a new version of Node.js is necessary, it's not very discoverable.

I added the following note to the release. Does that make it clear?

This is a maintenance release. An increasing amount of dependencies required a node version higher than the Node 10 version supported by semantic-relase@17. We decided to go straight to a recent Node LTS version because the release build is usually independent of others, requiring a higher node version is less disruptive to users, but helps us reduce the maintenance overhead.

@privatenumber
Copy link
Author

Thank you for updating the release notes! Much clearer IMO

If anyone wants, please feel free to address the doc updates.

Little busy right now but happy to tackle it if not updated later on.

@gr2m gr2m changed the title addressing v18 breaking changes Update documentation for GitHub Actions Setup Sep 24, 2021
@privatenumber
Copy link
Author

privatenumber commented Oct 5, 2021

@travi
I don't think this is completely resolved.

The problem this issue highlights is that people using npx semantic-release were hit by a breaking change when v18 was released.

The ask (open to other suggestions), is to recommend using npx 'semantic-release@^18.0.0' in the GitHub actions guide.

Will make a PR real quick.

@travi
Copy link
Member

travi commented Oct 5, 2021

@privatenumber i understand what you are saying, but i'm not fully on board with updating the github actions recipe for this specifically. we do mention this risk in our installation docs, so i would rather update the recipes to point back to that single source instead.

however, i also think that we should soften the recommendation. instead, i want to make the risks very clear and let each consumer of semantic-release make the decision for themselves. in most cases, the breaking changes that we introduce won't break the common use cases, especially if using the latest LTS version of node rather than a specific version. i fully agree that being surprised is a situation that we should help our users avoid, but i also dont want to encourage using a stale version of semantic-release when most cases likely don't need to be quite that careful.

i'm working on a docs update specifically around these topics, so i hope to improve how we communicate these points.

@privatenumber
Copy link
Author

Thanks for working on improving raising the risk awareness in the docs.

I didn't see that note on the install page. What do you think about highlighting it yellow or red as it's a risk warning?

So, would you like a PR that updates the GitHub Actions guide to point to the installation page docs?


in most cases, the breaking changes that we introduce won't break the common use cases, especially if using the latest LTS version of node rather than a specific version.

I haven't been using semantic-release long enough to speak about the other major releases. But a breaking change recently happened that requires users to be on a version of Node.js that's only 4 months old.

This broke CIs that use v12, which is in Maintenance LTS.

@travi
Copy link
Member

travi commented Oct 5, 2021

would you like a PR that updates the GitHub Actions guide to point to the installation page docs?

i plan to take a stab at something along these lines as part of the PR i linked to above. if you'd be willing to subscribe to that PR and provide feedback once i mark it as ready-for-review (it is still a bit disorganized as i try to clean up the individual parts that are a bit disconnected), your input would be welcome on whether it improves things or goes far enough.

a breaking change recently happened that requires users to be on a version of Node.js that's only 4 months old.

right, and we want to take the opportunity to improve our communication as a result. the contribution that was merged and resulted in closing this issue included what i think is the most important change to our recommendations for cases like the most recent breakage (dropping support of an EoL node version plus some additional versions that are not latest LTS). we will always make sure that we support at least the latest LTS version at the time of releasing a major version. however, we also plan to be more aggressive about dropping support for versions that are not the latest LTS. therefore, running the release phase of a CI pipeline with the latest LTS version (lts/* in the case of GitHub Actions) is often a better choice than pinning the major version.

obviously, breaking changes can include things that are more impactful than dropping a node version, but there are trade-offs to consider between risking a broken release temporarily vs continuing to use a stale version because an npx command hasnt been updated in a CI workflow for a while.

@privatenumber
Copy link
Author

Sounds good, thank you. I will take a look when ready for review.

Your reasoning for changing the Node.js version to LTS in the GitHub Actions example being the most important change sounds fine. However, I think in a lot of cases, Node versions are tied to codebase development.

As a reader, I wouldn't really think much of the node-version: 'lts/*' and change it the one my codebase needs instead. This is especially because there's no note next to it mentioning that semantic-release should always be run with an active LTS version.

This implicit requirement will be challenging to meet for codebases that need a lower version of Node.js, but use npm hooks (eg. prepublishOnly) to create the build (which will fail) triggered by semantic-release.

To be safe, I think it's important to further highlight the dangers of not specifying a version range. Afterall, that's the point of semver.

@travi
Copy link
Member

travi commented Oct 5, 2021

I think in a lot of cases, Node versions are tied to codebase development.

this is the other key point that we plan to highlight better in our docs update. many modern CI services enable using a different node version for the release phase than for the other phases in the pipeline. that way verification can be done against whatever matrix of versions are supported by the package and then do the release phase in a single controlled version. we plan to make it more clear that our recommendation is to use the latest LTS version specifically for the release phase, regardless of what is needed for the other phases.

but use npm hooks (eg. prepublishOnly) to create the build

hooks are another good call-out, especially since we typically recommend using prepack instead of any form of prepublish. not only has it not been changed, it also enables the hook to work in cases where you would run npm pack locally to test the package without publishing.

@privatenumber
Copy link
Author

Yes, I meant to specifically describe a scenario where users would upgrade the Node version for semantic-release CI, but realize they can't because they're building in a npm hook, and their build requires an older version of Node.

Anyway, thanks for your thoughtful responses. semantic-release has made my life so much easier and I'm glad to see the level of work and consideration put in. Looking forward to the doc improvements.

@travi
Copy link
Member

travi commented Oct 5, 2021

I meant to specifically describe a scenario where users would upgrade the Node version for semantic-release CI, but realize they can't because they're building in a npm hook, and their build requires an older version of Node.

i feel like i'm missing part of the point that you are trying to express. i consider a build step to be fairly closely tied to the release process, so i'm not thinking of a case where build steps would result in surprise if a team takes specific steps to configure a version of node for the release process. could you give a more concrete example if i haven't covered the scenario you have in mind?

@privatenumber
Copy link
Author

It would be challenging for projects with the following conditions to meet the active LTS requirement in the semantic-release CI:

  • Build tooling that requires an older (non-LTS) version of Node.js (eg. it could be something as minor as a legacy Webpack/Rollup/esbuild plugin)
  • The build requires the package.json version, and hence npm run build needs to be in the prepack hook

Ideas to work around this:

  • Specify the previous major version range of semantic-release in npx command
  • Install semantic-release locally to lock in the range in package.json
  • Find a way to migrate off of the legacy dependency that is coupled to the older version of Node.js
  • In the prepack hook, spawn a new shell with the older version of Node.js to produce a build

Specifying the major range in npx or package.json seems to be the simplest and least disruptive solution.

@github-actions
Copy link

🎉 This issue has been resolved in version 18.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants