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

Files are associated with PR instead of commit after rebase merge #2140

Closed
lukekarrys opened this issue Nov 26, 2023 · 0 comments · Fixed by #2141
Closed

Files are associated with PR instead of commit after rebase merge #2140

lukekarrys opened this issue Nov 26, 2023 · 0 comments · Fixed by #2141
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@lukekarrys
Copy link
Contributor

Environment details

  • OS: Darwin pieholden-m1.local 23.1.0 Darwin Kernel Version 23.1.0: Mon Oct 9 21:28:12 PDT 2023; root:xnu-10002.41.9~6/RELEASE_ARM64_T8103 arm64 arm Darwin
  • Node.js version: 20.7.0
  • npm version: 10.2.3
  • release-please version: 16.13.0

Steps to reproduce

  1. Create multiple commits for a manifest release with each commit targeting a different package. For example feat: ws1 changes workspace1/package.json and fix: ws2 changes workspace2/package.json.
  2. Create a PR with the above commits from a new branch to main
  3. Merge the pull request using Rebase and merge
  4. Run release-please on main to create the PR
  5. Observe that the body of the PR will show workspace2 having received a minor version bump.

Other Details

Here is a public PR where I observed this behavior: npm/cli#6609. There are a lot of unnecessary details in that PR but the specific bits are:

  • arborist got bumped to 6.3.0 via add new pkg fix command npm/cli#6626
  • That PR contained 3 commits, 2 of which changed files within workspaces/arborist/ with a fix: prefix
  • The other commit in the PR changed files outside of workspaces/arborist/ with a feat: prefix

I went through the code and found that this happens because files for a merge commit are backfilled from the PR instead of the commit. This is correct when a squash merge is performed, but I think it is incorrect for rebase merges.

@lukekarrys lukekarrys added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Nov 26, 2023
lukekarrys added a commit to lukekarrys/release-please that referenced this issue Nov 26, 2023
chingor13 added a commit that referenced this issue Nov 27, 2023
* fix: backfill files by commit for rebased PRs

Fixes #2140

* fix: always set commit PR from associated PRs

---------

Co-authored-by: Jeff Ching <chingor@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants