bump: skip PR checking when up to date or livecheck fails #18812
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
samford
commented
Nov 24, 2024
samford
commented
Nov 24, 2024
samford
force-pushed
the
bump-skip-pr-checking-when-up-to-date
branch
from
0137f2e
to
2ccded4
Compare
apainintheneck
approved these changes
Nov 25, 2024
`brew bump` will check for PRs related to a package even if the package version and livecheck version are the same. We're presumably only interested in related PRs when the livecheck version differs, so we can reduce GitHub API requests by skipping the check(s) when the versions are equal. We use `bump` in `autobump` workflows, so this should help with recent rate limiting issues (e.g., 3203 out of 3426 autobumped formulae were up to date in a recent run). This also reworks the output for duplicate PRs, making it clear when `bump` skipped checking PRs (as printing "none" would suggest that PRs were checked) and only printing the "Maybe duplicate" information when checked. This makes it a little easier to understand what `bump` has done internally from the output.
This expands test coverage for `BumpVersionParser`, bringing line coverage to 100% and branch coverage to 95.45%. This is the best we can do for branch coverage, as Sorbet will catch an invalid argument type before the method is executed, so we can't exercise the method in a test to get 100% coverage.
`brew bump` will check for PRs related to a package even if livecheck fails to identify new versions. This reworks related conditions to ensure that related PR checks are only run when livecheck returns version information.
samford
force-pushed
the
bump-skip-pr-checking-when-up-to-date
branch
from
2ccded4
to
1cbcc44
Compare
|
While tinkering with this some more, I noticed that |
samford
changed the title
apainintheneck
approved these changes
Nov 25, 2024
MikeMcQuaid
approved these changes
Nov 25, 2024
There was a problem hiding this comment.
Thanks @samford! Left some comments but for a follow-up PR as feels important to merge this as-is for now to help our rate limits.
| Maybe duplicate pull requests: #{maybe_duplicate_pull_requests || "none"} | ||
| EOS | ||
| if !args.no_pull_requests? && | ||
| (new_version.general != "unable to get versions") && |
There was a problem hiding this comment.
Would be good to extract unable to get versions (and, from below, none) into constants.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
brew stylewith your changes locally?brew typecheckwith your changes locally?brew testswith your changes locally?brew bumpwill check for PRs related to a package even if the package version and livecheck version are the same. We're presumably only interested in related PRs when the livecheck version differs, so we can reduce GitHub API requests by skipping the check(s) when the versions are equal. We usebumpinautobumpworkflows, so this should help with recent rate limiting issues (e.g., 3,203 out of 3,266 autobumped formulae were up to date in a recent run butbumpstill checked them for duplicate PRs).This also reworks the output for duplicate PRs, making it clear when
bumpskipped checking PRs (as printing "none" would suggest that PRs were checked) and only printing the "Maybe duplicate" information when checked. This makes it a little easier to understand whatbumphas done internally from the output.Lastly, this expands test coverage for
BumpVersionParser, bringing line coverage to 100% and branch coverage to 95.45%. This is the best we can do for branch coverage, as Sorbet will catch an invalid argument type before the#parse_versionmethod is executed, so we can't exercise the method in a test to get 100% coverage.