bump-cask-pr: fix duplicate PR checking with comma versions #16274
Conversation
|
Maybe I'm misunderstanding the fix here but it seems to still be missing the closed PRs for the example given in the issue. $ brew bump-cask-pr --dry-run --version 8u392-b08 semeru-jdk8-open
Error: These closed pull requests may be duplicates:
semeru-jdk8-open 8u392-b08 https://github.com/Homebrew/homebrew-cask-versions/pull/18540
semeru-jdk8-open 8u392-b08 https://github.com/Homebrew/homebrew-cask-versions/pull/18524
semeru-jdk8-open 8u392-b08 https://github.com/Homebrew/homebrew-cask-versions/pull/18514
Duplicate PRs should not be opened. Use --force to override this error.
$ brew bump-cask-pr --dry-run --version 8u392-b08,openj9-0.41.0 semeru-jdk8-open
==> Downloading https://github.com/ibmruntimes/semeru8-binaries/releases/download/jdk8u392-b08_openj9-0.41.0/ibm-semeru-open-jdk_x64_mac_8u392b08_openj9-0.41.
Already downloaded: /Users/kevinrobell/Library/Caches/Homebrew/downloads/a73924189f4d419f4e7f48dd47ef7d130f34c3ac127d8cfbdd8b0f6f95a07499--ibm-semeru-open-jdk_x64_mac_8u392b08_openj9-0.41.0.pkg
==> replace /version\s+["']8u392\-b08,openj9\-0\.41\.0["']/m with "version \"8u392-b08,openj9-0.41.0\""
==> brew audit --cask --online semeru-jdk8-open
==> brew style --fix semeru-jdk8-open.rb
==> try to fork repository with GitHub API
==> git add /usr/local/Homebrew/Library/Taps/homebrew/homebrew-cask-versions/Casks/semeru-jdk8-open.rb
==> git checkout --no-track -b bump-semeru-jdk8-open-8u392-b08-openj9-0.41.0 origin/master
==> git commit --no-edit --verbose --message='semeru-jdk8-open 8u392-b08,openj9-0.41.0' -- /usr/local/Homebrew/Library/Taps/homebrew/homebrew-cask-versions/Ca
==> git push --set-upstream FORK_URL bump-semeru-jdk8-open-8u392-b08-openj9-0.41.0:bump-semeru-jdk8-open-8u392-b08-openj9-0.41.0
==> git checkout --quiet -
==> create pull request with GitHub API (base branch: master) |
| @@ -174,6 +170,15 @@ def bump_cask_pr | |||
| GitHub.create_bump_pr(pr_info, args: args) | |||
| end | |||
|
|
|||
| sig { params(version: Cask::DSL::Version, cask: Cask::Cask).returns(Cask::DSL::Version) } | |||
| def shortened_version(version, cask:) | |||
| if version.before_comma == cask.version.before_comma | |||
There was a problem hiding this comment.
| if version.before_comma == cask.version.before_comma | |
| if version.before_comma == cask.version |
There was a problem hiding this comment.
That doesn't sound like it would ever be equal unless you are removing the comma in the bump.
The fix here was simply to share the existing logic used to title the commit/PR with the search functionality. They were previously not aligned so the search would of course not find the differently titled PR.
There was a problem hiding this comment.
I was seeing the same issue where it wasn't working now, but should moving forward as your comment below implies.
You will need to reset the local state of your repo to before the PRs were merged to see the effect of the fix. That matches the state the duplicate PRs were created (otherwise they would be empty diffs). Your example would probably fail on a non-dry-run from an empty commit. Maybe that particular scenario should be made more explicit to make it also fail on dry runs. |
There was a problem hiding this comment.
LGTM!
I was able to reproduce the expected behavior after reseting the cask versions repo back a few days.
$ brew bump-cask-pr --dry-run --version 8u392-b08,openj9-0.41.0 semeru-jdk8-open
Error: These closed pull requests may be duplicates:
semeru-jdk8-open 8u392-b08 https://github.com/Homebrew/homebrew-cask-versions/pull/18540
semeru-jdk8-open 8u392-b08 https://github.com/Homebrew/homebrew-cask-versions/pull/18524
semeru-jdk8-open 8u392-b08 https://github.com/Homebrew/homebrew-cask-versions/pull/18514
Duplicate PRs should not be opened. Use --force to override this error.
That's another good point. |
|
Great work @Bo98! |
Fixes #16271