Skip to content

feat(cicd): release-please #7831

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

Closed
wants to merge 1 commit into from
Closed

feat(cicd): release-please #7831

wants to merge 1 commit into from

Conversation

avivkeller
Copy link
Member

@avivkeller avivkeller commented Jun 3, 2025

This PR adds release-please to our CI/CD process. release-please will open a PR whenever a commit1 is pushed to main, with a new release of any modified packages.

It requires a RELEASE_PLEASE_TOKEN with contents: write and pull-requests: write permissions2.

My concern with this setup is that it's noisy. Every commit1 will cause this action to create a PR (unless a release PR is already open, in which case, the PR will be updated). Without this action, releases will silently be published as git SHAs on every commit1. Then again, we don't need to merge the PRs until there is something release worthy, which greatly reduces any noise

Footnotes

  1. Any commit that modifies a package, i.e. not apps/site changes 2 3

  2. For fine-grained tokens, this is "Public repositories", with "Repositories" set to "Contents: Read and write", "Pull requests: Read and write". For classic tokens, this is repo.public_repo

@Copilot Copilot AI review requested due to automatic review settings June 3, 2025 13:50
@avivkeller avivkeller requested review from a team as code owners June 3, 2025 13:50
Copy link

vercel bot commented Jun 3, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview Jun 3, 2025 1:50pm

Copy link
Member Author

Choose a reason for hiding this comment

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

This will need to be a required check

@avivkeller avivkeller added the infrastructure Issues/PRs related to the Repository Infra label Jun 3, 2025
Copy link

codecov bot commented Jun 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.46%. Comparing base (c11af00) to head (93e67e5).
Report is 20 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7831      +/-   ##
==========================================
- Coverage   75.48%   75.46%   -0.03%     
==========================================
  Files         101      101              
  Lines        8309     8309              
  Branches      218      218              
==========================================
- Hits         6272     6270       -2     
- Misses       2035     2037       +2     
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR integrates release-please into the CI/CD pipeline to automate release PR creation and package publishing.

  • Adds a release-please-config.json to specify which packages should be released.
  • Introduces a release-please.yml workflow to create release PRs on pushes to main and publish released packages.
  • Removes the old publish-packages.yml workflow and adds a lint check for PR titles.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
release-please-config.json Defines the packages for which release-please will generate releases.
.github/workflows/release-please.yml New workflow for creating releases and publishing packages.
.github/workflows/publish-packages.yml Deleted legacy publish workflow in favor of release-please.
.github/workflows/lint-pr-title.yml Added a workflow to enforce semantic PR titles.
Comments suppressed due to low confidence (1)

.github/workflows/release-please.yml:11

  • Setting permissions: {} revokes all default permissions, preventing necessary actions like checkout and PR creation. Specify minimal required scopes (e.g., contents: write, pull-requests: write) or remove this block to use the defaults.
permissions: {}

branches: [main]

concurrency:
group: release-${{ github.ref }}
Copy link
Preview

Copilot AI Jun 3, 2025

Choose a reason for hiding this comment

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

[nitpick] Using github.ref in the concurrency group may include the full ref path (e.g., refs/heads/main), leading to less-readable group names. Consider using github.ref_name to restrict it to the branch name.

Suggested change
group: release-${{ github.ref }}
group: release-${{ github.ref_name }}

Copilot uses AI. Check for mistakes.

Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

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

Could you document what pr-title-linting ask ? I mean some example

@avivkeller
Copy link
Member Author

It's enforcing conventional commits in the PR title

@AugustinMauroy
Copy link
Member

So what happens if we do foo(bar): quez?
In this case, foo is not known, so what will happen?

@avivkeller
Copy link
Member Author

So what happens if we do foo(bar): quez?

In this case, foo is not known, so what will happen?

The allowed scopes are https://github.com/commitizen/conventional-commit-types/blob/master/index.json. If an unknown scope is used, the action will fail, since release-please expects those scopes.

@ovflowd
Copy link
Member

ovflowd commented Jun 4, 2025

I'm not fond of an automated release bot; I do think our releases should be manual via manual workflow dispatch. We only need to push new releases when needed and we can do so anytime.

The manual workflow dispatch then could either already do the full release by itself or first a PR or whatever we prefer. I don't think we need an "evergreen" release model here. At the very least it would allow us to have a changelog and an actual versioning schema.

My issue with evergreen systems here is that it would require downstream (which ironically is upstream) Node.js's node repo to be constantly updated and make the releases meaningless and an arduous work to review. If this was a company only consuming internal packages, then this would be ideal imo.

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

So, to clarify, I'm -1 with the current state of the PR.

@bmuenzenmeyer
Copy link
Collaborator

I am currently -1 too and want to introspect more on how we got here. I don't see an issue cited, apologies if I missed it.

To be clear, it's not fair to Aviv to work on all this only to block it - and its also confusing to get cold PRs without first having alignment.

For example, I can think of at least three other ways we could automate publishing, so why this one? Some require more convention, some do not.

I'd rather we work to align on infra changes, before work begins.

@ovflowd
Copy link
Member

ovflowd commented Jun 4, 2025

To be clear, it's not fair to Aviv to work on all this only to block it - and its also confusing to get cold PRs without first having alignment.

I think this PR is a follow-up to #7777 which got no discussions either

@ovflowd
Copy link
Member

ovflowd commented Jun 4, 2025

That saaid, I agree I believe we should first achieve consensus on an approach before opening a PR? At least for this sort of things since it touches releasing, infra, etc? 🤔

@avivkeller
Copy link
Member Author

Sorry! It was on my TODO list, but +1 on reaches consensus first! I'll close this for now, and we can discuss in the dedicated issue.

@avivkeller avivkeller closed this Jun 4, 2025
@avivkeller avivkeller deleted the feat/release branch June 4, 2025 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Issues/PRs related to the Repository Infra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants