Skip to content

feat: incorporate changesets #266

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

Draft
wants to merge 10 commits into
base: dev
Choose a base branch
from
Draft

feat: incorporate changesets #266

wants to merge 10 commits into from

Conversation

onlywei
Copy link
Collaborator

@onlywei onlywei commented May 4, 2025

This PR introduces changesets to this repo as a way to automate the versioning and releasing of packages. Changesets replaces the need for conventional commits because that information is stored in the changeset file instead.

After this PR is merged, all pull requests that want to make a release should contain a "changeset file" that can be generated by running pnpm changeset locally from the root of the monorepo. The changeset CLI will ask you to enter the corresponding semver bump type and a description of your changes: feel free to enter a brief description via the command line and continue editing the generated Markdown file found inside of the .changeset directory in your editor of choice. When editing your changeset's Markdown file, you can leverage any of the features of GitHub flavored markdown! The description you provide in your changeset will be used to generate an entry in CHANGELOG.md and the release notes in a future release, so the more complete and descriptive the better.

Once a PR that contains a changeset file is merged to main (changesets works best if we stop using the dev branch), another PR will automatically created that consumes the changeset file. This PR will contain the following changes:

  1. The changeset file is deleted
  2. The package.json files of all packages to publish are automatically bumped according to the changeset files' wishes.
  3. CHANGELOG.md files are updated for all packages to publish.

Once the above automatically generated PR is merged, the package(s) are automatically published to npm AND a GitHub release is made with the correct git tags and release notes.

PERMISSIONS CHECKLIST

In order for all of the above to work, the following things must happen:

  • The changeset bot must be added to this repository. This will remind all pull requests to add changeset files automatically. I have already issued a request to the fastify org to do so.
  • A secret named NPM_TOKEN must be added to this repository's settings to allow it to publish to npm. This must be added under Settings > Secrets and variables > Actions > Repository Secrets
  • The "Allow GitHub Actions to create and approve pull requests" permission must be enabled. This can be found under Settings > Actions > General > Scroll to the bottom.

@onlywei onlywei requested a review from galvez May 4, 2025 20:48
Copy link

netlify bot commented May 4, 2025

Deploy Preview for agitated-mahavira-26f8f9 canceled.

Name Link
🔨 Latest commit 444869e
🔍 Latest deploy log https://app.netlify.com/sites/agitated-mahavira-26f8f9/deploys/6818d6d173a4a5000891dddb

@onlywei onlywei requested a review from climba03003 May 4, 2025 20:51
@onlywei onlywei marked this pull request as ready for review May 4, 2025 20:53
@galvez galvez changed the title Changesets feat: incorporate changesets May 4, 2025
@galvez galvez marked this pull request as draft May 4, 2025 21:05
@climba03003
Copy link
Member

climba03003 commented May 4, 2025

In order for this to work, we need to create an automated token from npm.
One critical problems is that, if I want to enforce the whitelist of token, I need to manfully input more than 5,000 of CIDR.

You can check the IPs used by Github Actions in below links https://api.github.com/meta

Possible solution would be create a dedicated machine for releasing only which is fixed IP.

@galvez
Copy link
Member

galvez commented May 4, 2025

Changed to draft to avoid any unwarranted merge before I've had a chance to review and absorb all of this. This is very substantial, a great improvement to the workflow, thanks @onlywei!

@onlywei
Copy link
Collaborator Author

onlywei commented May 4, 2025

In order for this to work, we need to create an automated token from npm. One critical problems is that, if I want to enforce the whitelist of token, I need to manfully input more than 5,000 of CIDR.

You can check the IPs used by Github Actions in below links https://api.github.com/meta

Possible solution would be create a dedicated machine for releasing only which is fixed IP.

@climba03003 Can you elaborate a little more? For my other packages I simply generated an access token from my npm account page and added it to the repos. What it special about the fastify org packages that requires a whitelist? I don't have permission to publish any fastify packages outside this repo anyway.

@climba03003
Copy link
Member

Can you elaborate a little more?

Whitelisting for certain IPs to protect the token is not misused in the other places outside of Github Actions.
It also prevent unauthorized usage when accidentally exposed to public.

For my other packages I simply generated an access token from my npm account page and added it to the repos. What it special about the fastify org packages that requires a whitelist?

It is not something special but security measure.

@zanmato
Copy link
Collaborator

zanmato commented May 5, 2025

Seems overly complicated, I like conventional commits though. But I agree that we should only use main or dev, not both

@onlywei
Copy link
Collaborator Author

onlywei commented May 5, 2025

Seems overly complicated, I like conventional commits though. But I agree that we should only use main or dev, not both

Conventional commits are good for repos that only have one package. However, conventional commits do not understand monorepos with multiple packages to publish. Plus we have to manually bump our version numbers to do manual publishing right now, which is error prone.

Copy link

changeset-bot bot commented May 5, 2025

⚠️ No Changeset found

Latest commit: 444869e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@galvez
Copy link
Member

galvez commented May 5, 2025

@onlywei I'd like to ensure we're able to manually do releases, and not have a CI-dependent setup.

I haven't read through it all yet, but this remains a possibility, right?

@onlywei
Copy link
Collaborator Author

onlywei commented May 5, 2025

@onlywei I'd like to ensure we're able to manually do releases, and not have a CI-dependent setup.

I haven't read through it all yet, but this remains a possibility, right?

Of course. Nothing prevents us from being able to manually release.

@onlywei
Copy link
Collaborator Author

onlywei commented May 5, 2025

@climba03003 Does the fastify org already have a precedent for setting up a dedicated machine such as a self-hosted github actions runner to do anything similar?

@climba03003
Copy link
Member

Does the fastify org already have a precedent for setting up a dedicated machine such as a self-hosted github actions runner to do anything similar?

AFAIK, currently no such infrastructure.
I am not sure if whitelisting IPs is a hard requirement, but it is good to have.

More discussion about automation release in fastify/workflows#112

@onlywei
Copy link
Collaborator Author

onlywei commented May 6, 2025

Does the fastify org already have a precedent for setting up a dedicated machine such as a self-hosted github actions runner to do anything similar?

AFAIK, currently no such infrastructure.

I am not sure if whitelisting IPs is a hard requirement, but it is good to have.

More discussion about automation release in fastify/workflows#112

I'm trying to find where I can make a feature request to the npm team for this, but the link on the npm website led me to ask my question here: https://github.com/orgs/community/discussions/158410

If this is not a strict requirement for the fastify org, then I think it's best to move forward without whitelisting and then add the whitelisting later when/if the npm team makes this enhancement.

@galvez
Copy link
Member

galvez commented May 20, 2025

@onlywei I'm trying to understand your first sentence:

This PR introduces changesets to this repo as a way to automate the versioning and releasing of packages. Changesets replaces the need for conventional commits because that information is stored in the changeset file instead.

What do you mean exactly conventional commits are not needed? I ask because in my understanding, conventional commits is just a suggested standard on how to name commit messages. What am I missing?

I want to remove the NPM publishing part out of the CI automation — until we have a solution that conforms with Fastify's security directives. But keep the CI automation for publishing release notes as drafts. Then we publish the packages manually, and also manually remove the draft status from the GH release. What do you think? @onlywei

@onlywei
Copy link
Collaborator Author

onlywei commented May 20, 2025

The point of conventional commits is to help automation decide whether a commit should make a patch, minor, or major version bump to a package. With changesets, this is no longer needed because that information is given in the changeset file instead.

I don't think we will ever be able to meet the security demand since it requires the npm team itself to make a feature addition, and there's no where I can find that we can talk to them. As @climba03003 said, it's not a hard requirement. Changesets is already used by big projects such as pnpm and GraphQL, so my opinion is that we should move forward without waiting for the npm team.

I'm not sure if the changeset GitHub action supports only drafting the release notes. Can you explain your concern here? If you want bigger control over the release notes, there are already a few opportunities to modify them:

  1. You can ask for edits to changeset files during PR code review.
  2. You don't have to click merge on the automated PR until ready: meaning you can make your own changeset file with your desired release notes and have the automated PR update itself to include that.

Perhaps a demo might help? Let me know.

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

Successfully merging this pull request may close these issues.

4 participants