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

Support "aggregate" mode in createGithubReleases config flag #193

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

dotansimha
Copy link

@dotansimha dotansimha commented Jun 27, 2022

Background for this PR

This PR allows users to specify createGithubReleases: aggregate in the config, in order to group a Changeset release flow into a single GitHub Release on GitHub.

This is useful for repositories with many packages and frequent releases, and prevents bloating GitHub Releases page.

Actual changes

  • Change createGithubReleases config flag to support true, false or aggregate mode.
  • Added support for creating a single GitHub Release for the entire Changeset publish command.
  • Added unit tests for the publish flow and a fixture matching the requirements

Preview

Due to the strict need to make GitHub Release's tag_name matches Git tag rules, I named it based on the execution time. Also the title. I'm not 100% sure about this and need a feedback on that part.

this is how it looks like on GitHub Releases page:

image

And with a custom name:

image

Related issues

@changeset-bot
Copy link

changeset-bot bot commented Jun 27, 2022

🦋 Changeset detected

Latest commit: cbc5ff3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@changesets/action Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@dotansimha
Copy link
Author

Also worth mentioning, that the entire GitHub Release depends today on Git tags created by the changeset publish command. Using --no-git-tag will cause this to be skipped.

Also, it is worth supporting GitHub Releases without having an attached GitHub tag maybe? Or at least to allow creating GitHub Releases without publishing any Git tag (i guess this could be done by improving the regex for the publication detection).

@dotansimha
Copy link
Author

changing to draft while I'm testing it in: https://github.com/dotansimha/changesets-experiments

@dotansimha dotansimha marked this pull request as draft June 28, 2022 14:30
@dotansimha dotansimha marked this pull request as ready for review June 28, 2022 15:48
@dotansimha
Copy link
Author

@Andarist any chance for a review/feedback on this one? Thanks 🤩

@paales
Copy link

paales commented Jun 30, 2022

Very excited for this one @dotansimha / @Andarist!

From a 'GitHub releases consumer' perspective this is great!

Without trying to discredit the work that has already been done. Maybe even the affected packages aren't as important for each change? Might something like this be a better layout?

### Patch Changes

- https://github.com/dotansimha/changesets-experiments/commit/13503ac703fa641530f7dba8137fae3a4781b65c: Test 3 [^1][^2]
- Some other change [^2]

[^1]: experimens-changesets-a@0.0.3
[^2]: experimens-changesets-b@0.0.3

Rendering as:

Patch Changes

Footnotes

  1. experimens-changesets-a@0.0.3

  2. experimens-changesets-b@0.0.3 2

@Andarist
Copy link
Member

@Andarist any chance for a review/feedback on this one? Thanks 🤩

I will get to it as soon as I can, I'm excited that you took a stab at this and I'm very much interested in this feature. I need to find a calm moment to review this with the clear head though and I probably won't have such this week.

@dotansimha
Copy link
Author

Btw, another idea to consider is to allow custom format for the title of the release (at the moment it's Release {DATE}).
The id can remain as release-{TIMESTAMP} to make sure it's always compatible with GitHub restrictions. The title can be more flexible and maybe allow we can have a title input that anyone can just pass (I think GitHub Actions provides a lot of useful env vars).

@dotansimha
Copy link
Author

I added githubReleaseName: string to the action inputs.

image

If someone wants to try it, it's available as:

      - name: Create Release Pull Request
        uses: dotansimha/changesets-action@262e957f99be29087d2b13afa32a5d579bf1d080
        with:
          publish: yarn release
          createGithubReleases: aggregate
          githubReleaseName: "Release ${{ steps.vars.outputs.sha_short }} (from ${{ steps.vars.outputs.branch }})"
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          NPM_TOKEN: ${{ secrets.NPM_TOKEN }}

@Andarist please let me know before merging so I'll remove the dist/action.js file - it's there to make testing of this PR easier :)

@dotansimha
Copy link
Author

dotansimha commented Jun 30, 2022

Without trying to discredit the work that has already been done. Maybe even the affected packages aren't as important for each change? Might something like this be a better layout?

I guess it's a matter of preference? we wanted to have the complete list of changes in one place, but I can see where just linking can become handy.
Maybe we can try to add that later as bodyFormat: 'link' | 'list' config flag?

@paales
Copy link

paales commented Jul 1, 2022

we wanted to have the complete list of changes in one place

Yes that is what we want as well.

I was suggesting to reformat the message itself to give each patch/minor/major more visibility and the packages less visibility.

In the next comment I'll post how that would look for your latest created release https://github.com/dotansimha/changesets-experiments/releases/tag/release-1656592601279

(I'll stop highjacking this thread and leave you guys to it)

@paales
Copy link

paales commented Jul 1, 2022

Minor Changes

Patch Changes

Footnotes

  1. experimens-changesets-a@0.1.0

  2. experimens-changesets-b@0.0.4

@dotansimha
Copy link
Author

@paales I created a new issue for this one: #195 , it might be relevant to the regular releases flow.

@Andarist
Copy link
Member

Andarist commented Jul 4, 2022

@paales I created a new issue for this one: #195 , it might be relevant to the regular releases flow.

Isn't this already same-ish for the regular release flow? I mean that a regular release flow always includes a single package in the release so the changes there are already grouped per release type.

src/index.ts Show resolved Hide resolved
"@changesets/action": minor
---

Allow to specify `createGithubReleases: aggregate`, in order to publish a single GitHub Release
Copy link
Member

Choose a reason for hiding this comment

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

My first idea when thinking about this feature was for this to be controlled by the .changeset/config.json and to also generate a single CHANGELOG.md for the involved packages. I have also wanted to only support this (at least at the start) for linked/fixed packages - as everything that gets released when using them shares a version so it simplifies tag and releases name creation.

What do you think about that idea? Any pros/cons that come to your mind when considering both approaches?

Copy link
Author

Choose a reason for hiding this comment

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

That's an interesting take. I was thinking about this with our team, and our biggest issue is the Releases on GitHub that's being bloated. Using only a single Release makes perfect sense for us (similar to the single PR created by Changesets).

Regarding the CHANGELOG, that's actually a different feature, no? I mean, you can still use aggregated release either with a unified CHANGELOG, or separate (per-package).
My take on CHANGELOG is that I like it within each package and not in a single place, because this way it's being part of the NPM packages and other tools (like Npm, or NPM stats services) can use it, and it's easier to have the scope of changes per each package in a single place. Usually if you are having an issue with a library, you'll look for the changes in that library/package specifically, and it's easier to find when it's located near the code.

I can see where merging CHANGELOGs might be useful for sure, especially if you are releasing all packages within a repo together (we did that in some libraries in the past, I think Babel is doing that?)
I guess this feature belongs to the cli more than the GH Action?

@dotansimha
Copy link
Author

dotansimha commented Jul 5, 2022

Isn't this already same-ish for the regular release flow? I mean that a regular release flow always includes a single package in the release so the changes there are already grouped per release type.

Yeah, I see what you mean, it's not that different, just a matter of formatting (UI? structure? What's the right terminology here? 😆 )

I think there's room for improvements in the markdown anyway, but I guess that's not part of the scope of the GH Action? (isn't that calculated as part of the CLI?)

@dotansimha dotansimha requested a review from Andarist July 10, 2022 08:27
@dotansimha
Copy link
Author

@Andarist any chance for a final review here? :)

@dotansimha
Copy link
Author

@Andarist reminder ;)

@luizstacio
Copy link

Really needed! Looking forward to the release...

action.yml Show resolved Hide resolved
@luizstacio
Copy link

@dotansimha I also added a githubTagName configuration and made the change I mentioned on the README. dotansimha#1. @Andarist considered merging these updates also with the current PR.

@dotansimha
Copy link
Author

@luizstacio thank you!

@dotansimha
Copy link
Author

Btw, we are testing it in a few repos, and so far it seems to work great :)

here's an example: https://github.com/dotansimha/graphql-code-generator/releases/tag/release-1659968285830

@luizstacio
Copy link

Btw, we are testing it in a few repos, and so far it seems to work great :)

here's an example: https://github.com/dotansimha/graphql-code-generator/releases/tag/release-1659968285830

We are also using it here: https://github.com/FuelLabs/fuels-ts/releases/tag/v0.11.0.

Why we consider this approach

We publish all our packages to npm in different packages. But when it arrived at the GitHub releases/tags, it didn't make sense for us to generate separated releases/tags. Why;

  • The idea of a monorepo is to have a single repository; with this in mind, a single tag and a single release cover all the packages.
  • The individual package release/tag generated by the changeset don't have any difference, only the changeset text. The source code (tar) are all redundant.
    • It would make more sense if each release also contained the tar file generated by npm pack and each release was also a GitHub package. But it's not something we are looking for, maybe other teams.

@emmenko
Copy link
Contributor

emmenko commented Aug 10, 2022

@dotansimha Thanks for the awesome contribution and the work you put on it!

We'll be testing this in one of our repo as well (in case it helps with testing and feedback) 🤗

@dotansimha
Copy link
Author

dotansimha commented Aug 11, 2022

@emmenko we noticed a minor issue where empty pre-releases are created. I fixed it and updated it. Also, I published a temporary versioned fork:

      - name: Create Release Pull Request
        uses: dotansimha/changesets-action@v1.3.3
        with:
          publish: yarn release
          createGithubReleases: aggregate
          githubReleaseName: "Release ${{ steps.vars.outputs.sha_short }} (from ${{ steps.vars.outputs.branch }})"
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          NPM_TOKEN: ${{ secrets.NPM_TOKEN }}

@emmenko
Copy link
Contributor

emmenko commented Aug 31, 2022

FYI: we've been testing the aggregate mode and so far it's been working great 💯

@ghost
Copy link

ghost commented Aug 31, 2022

This would be great addition

@dotansimha
Copy link
Author

Just a heads up: we noticed an issue with the body limit of GitHub, in huge releases (multiple changesets, ~30 packages that are all effected), we sometimes get to the limit.
Still not sure if that's a real concern or a very rate edge-case...

@emmenko
Copy link
Contributor

emmenko commented Sep 8, 2022

Ah it might be similar to #157

I think it should be tackled (if possible), we have a repo with ~100 packages.

@luizstacio
Copy link

luizstacio commented Sep 8, 2022

@Andarist will be great to have a view here and understand if this is in the interest of changesets, if not, the community on this PR. Should create a fork and add it to the marketplace to have better-centralized maintenance and not need to use forks...

cc: @dotansimha

@itsdouges
Copy link

Hey folks I'd be keen to see this released - what's currently missing? Review from @Andarist? Are there any code changes I can help with?

For my use case I have a monorepo that is released together and not all releases have actual changes. Github releases start getting spammy when there isn't any useful infomation in some version bumps. To this end I think grouping "fixed/linked" packages together into a single release would be quite logical (since they're in essence, released as a single entity!).

@enisdenjo
Copy link

Hello everyone, just wanted to ping you guys that the dotansimha/changesets-action fork has released v1.5.0 which includes the following:

Since this PR is somewhat stuck, you may use the fork (like showcased here).

@Y0moo
Copy link

Y0moo commented May 1, 2023

Hey there! How is the progress with this feature? We are looking forward to using it in our project. Can help if any code changes are needed.

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