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

feat: Introduce componentNoSpace parameter (Removes whitespace character from ${component} title pattern) #2330

Merged
merged 10 commits into from
Sep 16, 2024

Conversation

Kiruyuto
Copy link
Contributor

@Kiruyuto Kiruyuto commented Jul 8, 2024

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 #1626 🦕

Introduces new parameter (componentNoSpace) which removes (space) character from component effectively allowing to use component as a scope or in any other "complex" scenario, eg.:

"pull-request-title-pattern": "chore(${component}): ${version}"

Prior to this change attempting to do so resulted in this kind of behaviour
image

@Kiruyuto Kiruyuto requested review from a team as code owners July 8, 2024 23:38
@product-auto-label product-auto-label bot added the size: s Pull request size is small. label Jul 8, 2024
@simonostendorf
Copy link

I would be interested that this PR will be merged. What else needs to happen for that?

@Kiruyuto
Copy link
Contributor Author

Kiruyuto commented Sep 8, 2024

I would be interested that this PR will be merged. What else needs to happen for that?

A stroke of luck that some maintainer sees this PR perhaps? I bet folks at google are busy but given the fact that last non-automated commit was made over 3 months ago it doesn't bode well 👀

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.

Thank you for this, it will potentially visibly break users and disrupt parsing as the title does hold data for release-please.

Can we introduce a new template parameter (e.g. componentNoSpace) and handle both? In the next major version, we can "fix" this by making the breaking change.

@Kiruyuto Kiruyuto changed the title fix: Remove whitespace character from ${component} [WIP] feat: Introduce componentNoSpace parameter (Removes whitespace character from ${component} title pattern) Sep 11, 2024
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: s Pull request size is small. labels Sep 11, 2024
@Kiruyuto Kiruyuto changed the title [WIP] feat: Introduce componentNoSpace parameter (Removes whitespace character from ${component} title pattern) feat: Introduce componentNoSpace parameter (Removes whitespace character from ${component} title pattern) Sep 11, 2024
@Kiruyuto
Copy link
Contributor Author

Kiruyuto commented Sep 11, 2024

Hey,
Thanks for your review. As per your instructions, I added a new config option/parameter, so it shouldn't mess up existing configurations, pipelines and whatnot.
Although I quickly tested the change, I might have missed a scenario where it might break up a thing or two, so lmk if you come up with something.

I've also tested it in a live repo, which you can see there:
Config file: https://github.com/Kiruyuto/rp-test/blob/master/release-please-config.json
PRs: https://github.com/Kiruyuto/rp-test/pulls

Below are the commands I used to bootstrap and test that repo

CLI commands
release-please bootstrap --token=[REDACTED] --repo-url=Kiruyuto/rp-test --release-type=node
npm run compile && node build/src/bin/release-please.js release-pr --token=[REDACTED] --repo-url=Kiruyuto/rp-test

@chingor13 chingor13 added this pull request to the merge queue Sep 16, 2024
Merged via the queue into googleapis:main with commit 0a2d5c6 Sep 16, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide ${component} without whitespace of the beginning
3 participants