-
Notifications
You must be signed in to change notification settings - Fork 335
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
Rename GitHub branch early releases to “previews” #4000
Conversation
6494cad
to
e549eb7
Compare
e549eb7
to
2102013
Compare
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.
Nice! Just a little suggestion for the name of the script, a missing article and a little formatting improvement 😊
package.json
Outdated
@@ -19,7 +19,7 @@ | |||
"start": "npm run dev", | |||
"build-release": "./bin/build-release.sh", | |||
"publish-release": "./bin/publish-release.sh", | |||
"pre-release": "./bin/pre-release.sh", | |||
"preview": "./bin/preview.sh", |
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.
suggestion I think it'd be worth updating that script name to publish-preview
(as preview
on its own may lead to thinking it starts some local preview).
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.
Hmm, wouldn't it be build-preview
as it's paired with build-release
?
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'll go with publish-preview
since that's the task (publishing to a GitHub branch)
But yeah the script is mostly similar to build-release
We’ll refer to ad-hoc GitHub branch releases as “previews” and only use the term “pre-release” for early releases that are also published to npm
We add the `npm version` flag `--allow-same-version` to allow existing previews to updated with the same version number We can’t use `npm pkg get version` due to output differences between npm v8 and v9, plus a “double output” bug only seen in v9: * npm/cli#5481
2102013
to
5a5dacd
Compare
@@ -50,7 +50,8 @@ | |||
"build:package": "gulp build:package --color", | |||
"build:release": "gulp build:release --color", | |||
"build:stats": "npm run stats --workspace govuk-frontend-stats", | |||
"dev": "gulp dev --color" | |||
"dev": "gulp dev --color", | |||
"version": "echo $npm_package_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.
Nice one! That's clever 😍
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.
Forgot to approve as I posted that last comment 🤦🏻
Missed from code review feedback: * #4000 (comment)
Rename GitHub branch early releases to “previews”
This PR renames our publishing-a-pre-release.md process:
To the term "preview" rather than "pre-release":
We've decided to refer to ad-hoc GitHub branch releases as “previews” since they're not published to npm
Package version for previews
All preview builds now run
npm version
to suffix the branch name in the package.json"version"
We can now identify preview versions of GOV.UK Frontend when deployed as part of: