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: pass pull request header and footer options to merge plugin #2143

Merged
merged 5 commits into from
Dec 11, 2023

Conversation

lukekarrys
Copy link
Contributor

@lukekarrys lukekarrys commented Nov 27, 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 #2142 🦕

Details

This works by looping through repositoryConfig and getting the first set value for both pullRequestHeader and pullRequestFooter. This means they could possibly come from different components in a manifest release.

Alternate Solutions

I thought about only using config values from repositoryConfig[ROOT_PROJECT_PATH]. I didn't have strong feelings either way so I went with the solution I felt would be less surprising. But I'd be open to reworking the PR to this alternate solution.

@lukekarrys lukekarrys requested review from a team as code owners November 27, 2023 02:23
@product-auto-label product-auto-label bot added the size: s Pull request size is small. label Nov 27, 2023
Copy link
Contributor

@chingor13 chingor13 left a comment

Choose a reason for hiding this comment

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

Thanks for this! The feature seems reasonable, but this one has a bit more complex logic and could use a unit test to demonstrate the example and to ensure we don't regress in the future.

@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Nov 30, 2023
@lukekarrys
Copy link
Contributor Author

Thanks @chingor13! I added a test case where it grabs the header from the first package and the footer from the second package another, while also ignoring the footer from the second package. Let me know what you think.

@lukekarrys
Copy link
Contributor Author

I didn't realize I left the tests in a failing state. Just pushed a fixup that should be passing now.

Copy link
Contributor

@chingor13 chingor13 left a comment

Choose a reason for hiding this comment

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

Thanks!

@chingor13 chingor13 merged commit e848100 into googleapis:main Dec 11, 2023
11 checks passed
@lukekarrys lukekarrys deleted the lk/merge-plugin-header-footer branch December 11, 2023 18:41
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.

Merge plugin does not use pull-request-header and pull-request-footer config values
2 participants