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

fix: backfill files by commit for rebased PRs #2141

Merged
merged 3 commits into from
Nov 27, 2023

Conversation

lukekarrys
Copy link
Contributor

@lukekarrys lukekarrys commented Nov 26, 2023

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #2140 🦕

This has the effect of not associating a PR with any of the commits from a PR that is rebased and merged. Previously it would associate the PR only with the first commit, but this would also add all the files from the PR to that first commit.

A different (or future) solution would be to decouple connecting PRs and files with commits. I considered that but thought that solution would increase the scope too much. I'm open to reworking the PR in that direction though.

@lukekarrys
Copy link
Contributor Author

This has the effect of not associating a PR with any of the commits from a PR that is rebased and merged.

After writing that comment, I think I found a simple solution to this. I added another commit to this PR that always sets commit.pullRequest if there are any associatedPullRequests.

I can revert this commit if this is not a desired change, or make it a separate PR.

I would love for this additional change to land since in the npm CLI we always run an additional API request for each commit to get its associated PRs for this reason.

@chingor13 chingor13 merged commit ee6d82f into googleapis:main Nov 27, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Files are associated with PR instead of commit after rebase merge
2 participants