-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Update maven version pattern for new parser #11767
Merged
kbukum1
merged 4 commits into
main
from
jonabc/update-maven-version-pattern-for-new-parser
Mar 12, 2025
Merged
Update maven version pattern for new parser #11767
kbukum1
merged 4 commits into
main
from
jonabc/update-maven-version-pattern-for-new-parser
Mar 12, 2025
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
a requirement like `> 1.2.3+0.1.1` can be provided as part of an ignore condition
a requirement like `> 1.2.3+0.1.1` can be provided as part of an ignore condition
The maven version parser was broadened in #10704, however VERSION_PATTERN was not updated to match. Looking at the version parser and the details of maven's ComparableVersion https://maven.apache.org/ref/3.5.2/maven-artifact/apidocs/org/apache/maven/artifact/versioning/ComparableVersion.html, it looks like "unlimited number of version components" means that the regex pattern can be simplified.
2aa8372
to
515970a
Compare
honeyankit
previously approved these changes
Mar 10, 2025
kbukum1
previously approved these changes
Mar 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Will be deployed after #11764 is deployed. |
Base automatically changed from
jonabc/gradle-support-ruby-requirement-ops
to
main
March 12, 2025 19:17
The base branch was changed.
Going to deploy after conflict is fixed. |
kbukum1
approved these changes
Mar 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
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.
What are you trying to accomplish?
The maven version parser was recently broadened to support a newer version format, however the
Dependabot::Maven::Version::VERSION_PATTERN
was not updated to match. I'm updating the pattern to be more flexible in order to match the new version parsers behavior.Anything you want to highlight for special attention from reviewers?
This branch and PR are based on #11764 because the added requirement test case uses a dynamic version, and the only way I could find to keep the full version string intact is using a predefined ruby-style requirement string which needs the linked PR to work.
I think this change should be safe. I looked at maven's ComparableVersion, which specifies
unlimited number of version components
as a feature. TheDependabot::Maven::VersionParser
implementation also looks to allow any number of version parts. All of that together means that the regex pattern can be simplified to look for any number of trailing version parts.How will you know you've accomplished your goal?
@dependabot ignore this version
should work with for a maven dependency version like25-ea+5.a0
Checklist