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

triage: update bump-formula-pr label logic #153925

Conversation

chenrui333
Copy link
Member

@chenrui333 chenrui333 commented Nov 10, 2023

For improving the signal to include more formulae for autobump, Created by brew bump is a better signal than Created by bump-formula-pr for the labeling.

Examples like:

  • minio or minio-mc
  • luagit

For improving the signal to include more formulae for autobump, `Created by brew bump` is a better signal than `Created by bump-formula-pr` for the labeling.

Signed-off-by: Rui Chen <[email protected]>
@chenrui333 chenrui333 added the CI-syntax-only Change only affects brew syntax, not the install. Only run syntax CI. label Nov 10, 2023
@chenrui333 chenrui333 requested review from MikeMcQuaid and a team as code owners November 10, 2023 16:48
@github-actions github-actions bot added workflows PR modifies GitHub Actions workflow files automerge-skip `brew pr-automerge` will skip this pull request labels Nov 10, 2023
@chenrui333 chenrui333 enabled auto-merge November 10, 2023 17:41
@Bo98
Copy link
Member

Bo98 commented Nov 10, 2023

Bit weird to have a label called bump-formula-pr and to exclude bump-formula-pr.

@Bo98
Copy link
Member

Bo98 commented Nov 10, 2023

Is the motivation behind this about the shorter stale times that brew bump and brew bump-formula-pr currently have?

@chenrui333 chenrui333 disabled auto-merge November 10, 2023 19:11
@chenrui333
Copy link
Member Author

Bit weird to have a label called bump-formula-pr and to exclude bump-formula-pr.

yeah, I can change the label if that helps :)

@chenrui333
Copy link
Member Author

Is the motivation behind this about the shorter stale times that brew bump and brew bump-formula-pr currently have?

not really, the motivation is mostly because brew bump-formula-pr is not a good signal for helping out autobump process (because it can be triggered manually with bunch overrides) I have attached some examples

@MikeMcQuaid
Copy link
Member

Bit weird to have a label called bump-formula-pr and to exclude bump-formula-pr.

Agreed. Should have one label that includes both and consider renaming existing label.

@@ -221,7 +221,7 @@ jobs:
keep_if_no_match: true

- label: bump-formula-pr
pr_body_content: Created with `brew bump-formula-pr`
pr_body_content: Created with `brew bump`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pr_body_content: Created with `brew bump`
pr_body_content: Created with `brew bump

will cover all cases

Copy link
Member Author

Choose a reason for hiding this comment

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

oh, right now it does cover all cases, it is just the bump-formula-pr can be a false signal when generating autobump list.

Copy link
Member

Choose a reason for hiding this comment

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

it is just the bump-formula-pr can be a false signal when generating autobump list.

sorry, I don't understand what this means: can you elaborate a lot more?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, right now, folks can use either bump --open-pr or bump-foruma-pr to create the formula bump PRs, while they both can do the same thing, the bump-formula-pr can take lots of parameters overrides, which would be quite manual rather than something can be done by bot (minio and luagit are examples on this), while AFAIK you can override parameters via bump --open-pr, which means, the PRs using it can be also done by bot.

Let me know if that makes better sense.

Copy link
Member

Choose a reason for hiding this comment

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

I understand the differences between the two approaches but not how that affects what should be labelled what or why the contents of the label affects anything?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, i have to say, it seems a bit tricky about getting this perfect 😄

Now, I am gonna manually use cannot-autobump label to help with the autobump signal.

@chenrui333
Copy link
Member Author

Closing this PR for now to use cannot-autobump label to help with autobump process

@chenrui333 chenrui333 closed this Nov 20, 2023
@chenrui333 chenrui333 deleted the triage-update-bump-formula-pr-label-logic branch January 22, 2024 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autobump automerge-skip `brew pr-automerge` will skip this pull request CI-syntax-only Change only affects brew syntax, not the install. Only run syntax CI. workflows PR modifies GitHub Actions workflow files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants