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

Add pre-release documentation with version numbers #3977

Merged
merged 12 commits into from
Nov 21, 2023

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Jul 19, 2023

This PR adds pre-release documentation and sets a version number using npm version

Pre-release version numbers

For example, for early preview of v5.0.0 from branch main we could:

  • Use version premajor to bump from v4.7.0 to v5.0.0-beta.0
  • Use version preminor to bump from v4.7.0 to v4.8.0-beta.0
  • Use version prepatch to bump from v4.7.0 to v4.7.1-beta.0
  • Use version prerelease to bump from v5.0.0-beta.0 to v5.0.0-beta.1

This determines the npm version command to update package.json and package-lock.json:

npm version <VERSION NUMBER> --preid beta --no-git-tag-version --workspace govuk-frontend

Unlike previews, pre-releases must pick patch/minor/major bumps so it's hard to automatically version

@colinrotherham colinrotherham added the documentation User requests new documentation or improvements to existing documentation label Jul 19, 2023
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3977 July 19, 2023 07:44 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3977 July 19, 2023 08:13 Inactive
@36degrees
Copy link
Contributor

Thanks for picking this up!

These existing prerelease docs are focused on releasing 'to GitHub' – in step 6 when we run npm run pre-release it creates a 'filtered' orphaned branch with the contents of the package we would publish to npm.

Is there value in prereleases to GitHub having a pre-release version number?

I'm wondering if we're conflating two things that should be kept separate? Maybe we need different terms for 'pre-releases pushed to GitHub' (which I think are still useful for e.g. exploratory testing) and 'pre-releases to npm' which are more like release candidates?

@colinrotherham
Copy link
Contributor Author

colinrotherham commented Jul 19, 2023

Is there value in prereleases to GitHub having a pre-release version number?

@36degrees Thought we should put @querkmachine's versioning work into action

When we run npm run pre-release it picks up the new "version" from package.json

It'll be confusing if our first pre-release identifies the version as 4.6.0 won't it?

CSS version

getComputedStyle(document.body)
  .getPropertyValue('--govuk-frontend-version')

JS version

await import('/javascripts/govuk-frontend.min.js')
  .then(({ version }) => console.log(version))

So instead of seeing 4.6.0 we'll see 5.0.0-rc.0 🙌

I'm wondering if we're conflating two things that should be kept separate? Maybe we need different terms for 'pre-releases pushed to GitHub' (which I think are still useful for e.g. exploratory testing) and 'pre-releases to npm' which are more like release candidates?

Is it the -rc suffix from --preid rc that worries you? We can change it

Have a look down the typescript versions "Tag" column for other examples:

@latest
@next
@dev
@beta
@rc
@insiders

Not for this PR, but publishing to npm is just one script tweak away

@36degrees
Copy link
Contributor

36degrees commented Jul 19, 2023

I think it makes sense to use incremental version numbers (rc.0, rc.1) when we're working towards a release and doing pre-releases from the main branch.

But, we also sometimes create GitHub-based pre-releases from feature branches just to test something. Effectively, these are 'throw-away' releases just to spike something, try something out in the Design System repo or Prototype Kit etc.

I can see how it'd be useful to have a different version identifier in those instances, but I don't think it should be incremental or tied to any specific version for a couple of reasons.

  1. When I'm testing something out on a feature branch, I may not yet know how it's going to be released. Following this process, I have to make a decision about something that I don't care about right now.
  2. Because we 'throw it away', the incremented version number never hits the main branch. So if we create multiple releases from feature branches, they will all be called v5.0.0-rc.0 despite being distinct. The eventual npm pre-release will also be called v5.0.0-rc.0 but will be different yet again.

@colinrotherham
Copy link
Contributor Author

colinrotherham commented Jul 19, 2023

Ah my fault, I saw the Update a pre-release steps and thought they were iterative

So that all our v5 pre-release users can clearly identify which iteration they're on, can we support both?

npm version 5.0.0-adhoc --no-git-tag-version --workspace govuk-frontend
npm version 5.0.0-colin --no-git-tag-version --workspace govuk-frontend
npm version 5.0.0-ollie --no-git-tag-version --workspace govuk-frontend

@36degrees
Copy link
Contributor

For 'actual' pre-releases to npm, I have no issue using the rc.x suffix.

This is just about pre-releases from feature branches. Using the branch name for the version number might be a good option?

Given it seems like we're getting in a muddle I'm starting to think we do need distinct terms and documentation…

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3977 July 19, 2023 14:20 Inactive
@colinrotherham
Copy link
Contributor Author

colinrotherham commented Jul 19, 2023

Yeah that works, I've updated and pushed the docs

We need the branch name as a suffix like 5.0.0-ollie if that's alright?

I've added incremental pre-releases following the Update a pre-release steps as an alternative.

This is just about pre-releases from feature branches. Using the branch name for the version number might be a good option?

Would certainly align with "working in the open" if every GitHub pre-release was published for easy npm install too

Let's ask @domoscargin for feedback as well

@domoscargin
Copy link
Contributor

I think the branch name as a suffix makes sense, mostly. But how would we ensure consistency there? If I'm doing a quick throwaway thing that isn't yet targeting a release, I'd probably have to go with either:

  1. Using the current version, which is unlikely to be accurate
  2. Using the next version, but what if you use, say, 5.0.0, for something that ends up in 4.8.0?
  3. Using a random version number
  4. Or just using the branch name and skipping version number

Not a particularly big concern, but could end slightly messy.

Other than that, all the rc stuff for "real" pre-releases sounds good!

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3977 July 19, 2023 15:39 Inactive
@colinrotherham
Copy link
Contributor Author

colinrotherham commented Jul 19, 2023

The worry at the moment is we use 1) the "current version" so the v5 pre-release will be:

For now, I've pushed up a more generic version where <VERSION NUMBER> is up to the team still:

npm version <VERSION NUMBER> --no-git-tag-version --workspace govuk-frontend

At pre-release time, if we want to incorporate the branch name that's fine

@colinrotherham
Copy link
Contributor Author

For our v5 pre-releases we’re probably going to have at least three? No doubt more

  • v5.0.0-rc.0 — Initial pre-release, this week?
  • v5.0.0-rc.1 — Incremental pre-release, removal of init() methods
  • v5.0.0-rc.2 — Incremental pre-release, API changes to throw errors

But for Design System previews we'll likely do the same for new typography scale pre-releases

Can we think of a naming convention that works with npm version for both of these?

@romaricpascal
Copy link
Member

Following this morning's dev catch-up and further slack discussions, we're going to clearly separate the concept of "pre-release" (to npm) from "preview branches" (on a branch on Github, which we currently call pre-release 😱 ).

We need to do a little more research on how to handle npm pre-releases (especially on how to handle the version bump, suffix or suffixes to use, changelog, docs...).

We can already start updating our documentation for releasing to a preview branch, though.

@36degrees
Copy link
Contributor

I think this needs updating so that any changes to accommodate pre-releases are made to our existing release documentation instead? Is that right?

@colinrotherham
Copy link
Contributor Author

colinrotherham commented Jul 25, 2023

I think this needs updating so that any changes to accommodate pre-releases are made to our existing release documentation instead? Is that right?

@36degrees Yeah let's get it back into draft as it's not needed for v5.0.0 preview anymore

I'll fix the conflicts, push it up, but can work on it another day

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3977 July 25, 2023 14:38 Inactive
@colinrotherham colinrotherham marked this pull request as draft July 25, 2023 14:39
@colinrotherham colinrotherham changed the title Update pre-release documentation for version numbers Add pre-release documentation with version numbers Jul 27, 2023
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3977 October 11, 2023 15:00 Inactive
@github-actions
Copy link

github-actions bot commented Oct 11, 2023

📋 Stats

File sizes

File Size
dist/govuk-frontend-5.0.0-beta.1.min.css 114 KiB
dist/govuk-frontend-5.0.0-beta.1.min.js 37.93 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 77.58 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 72.89 KiB
packages/govuk-frontend/dist/govuk/all.mjs 3.86 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs 359 B
packages/govuk-frontend/dist/govuk/govuk-frontend.min.css 113.53 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 38.11 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.38 KiB

Modules

File Size
all.mjs 69.22 KiB
components/accordion/accordion.mjs 21.47 KiB
components/button/button.mjs 4.5 KiB
components/character-count/character-count.mjs 21.05 KiB
components/checkboxes/checkboxes.mjs 5.63 KiB
components/error-summary/error-summary.mjs 5.89 KiB
components/exit-this-page/exit-this-page.mjs 15.89 KiB
components/header/header.mjs 3.71 KiB
components/notification-banner/notification-banner.mjs 4.34 KiB
components/radios/radios.mjs 4.63 KiB
components/skip-link/skip-link.mjs 3.61 KiB
components/tabs/tabs.mjs 9.47 KiB

View stats and visualisations on the review app


Action run for 7792ae8

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3977 October 23, 2023 10:33 Inactive
@romaricpascal
Copy link
Member

Updated with the following changes, based on our last pre-release runs:

  • Removing a few "if you're publishing a beta" statements no longer necessary as we merge both internal and beta on main
  • Generalise how we check we're running the right node version, as not everyone runs nvm
  • Move the update of the version to before creating the branch. We don't commit until we run the build-package script, so we may as well let npm compute the new version number. It also allows to get a copy-pastable command for creating the branch, as we can read the new version number
  • Added a statement to clarify that prerelease would also move the version from -internal.X to -beta.0 and a note that it'd reset the last number to .0
  • Added a note to log out from npm in the browser if the command line sent you there for logging in.

- Clarifies a couple of instructions
- Harmonises the capitalisation of GitHub
- Add link to CHANGELOG.md when needing to make the release notes
@romaricpascal
Copy link
Member

@colinrotherham I've updated based on what you spotted during this morning's pre-release, and tried to address your two comments 😊

docs/releasing/publishing-a-pre-release.md Outdated Show resolved Hide resolved
docs/releasing/publishing-a-pre-release.md Outdated Show resolved Hide resolved
docs/releasing/publishing-a-pre-release.md Show resolved Hide resolved
docs/releasing/publishing-a-pre-release.md Outdated Show resolved Hide resolved
docs/releasing/publishing-a-pre-release.md Outdated Show resolved Hide resolved
docs/releasing/publishing-a-pre-release.md Outdated Show resolved Hide resolved
- commit the changes
- push a branch to GitHub

You will now be prompted to continue or cancel.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might seem pedantic, but should we give the user an action here? Similar to step 5 of "Publish a release to npm"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we can here. In the npm part, step 4, we can guide the users towards pressing N as we know they'll have to change from 'latest' to 'next' or 'internal'. This is more like the step 5 of npm where we need the person driving to have a pause and consider whether to continue, rather than automatically go "'y', let's go!"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was looking to make this look a bit more like step 5 in "Publish a release to npm" to clarify what happens if they continue, and what happens if they cancel.

At the moment, it's slightly unclear whether the list of things that get done when we run the command happen before or after the prompt.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see, yeah, I can add the same clarification here, good shout 😊


This step will create a ZIP file containing the release in the root of your govuk-frontend git directory. You will need this file when creating the GitHub release.

It will also automatically create a tag in GitHub which you can use to create a GitHub release in the following section.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pedantic: it's not super clear what "it" is referring to here, since we're talking about a file just before, instead of "this step".

docs/releasing/publishing.md Outdated Show resolved Hide resolved
@romaricpascal
Copy link
Member

@domoscargin I've addressed your suggestions and replied when I didn't make an update. Ready for another review 😊

Copy link
Contributor

@domoscargin domoscargin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romaricpascal Looking good. I've left one comment where I think we can still be clearer, but I wouldn't consider it blocking - we can always tweak this as we do more pre-releases.

Co-authored-by: Brett Kyle  <brett.kyle@digital.cabinet-office.gov.uk>
Co-authored-by: Colin Rotherham <colin.rotherham@digital.cabinet-office.gov.uk>
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3977 November 20, 2023 12:57 Inactive
@romaricpascal romaricpascal merged commit 86a748b into main Nov 21, 2023
44 checks passed
@romaricpascal romaricpascal deleted the docs-pre-release-version branch November 21, 2023 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation User requests new documentation or improvements to existing documentation
Projects
Development

Successfully merging this pull request may close these issues.

5 participants