-
Notifications
You must be signed in to change notification settings - Fork 1
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
fix(semver): replace semver library with translation from node-semver #21
base: main
Are you sure you want to change the base?
Conversation
return compareIdentifiers(this.patch, other.patch); | ||
} | ||
|
||
public int comparePre(SemVer other) { |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
The cyclomatic complexity of this method is 13. By comparison, 98% of the methods in the CodeGuru reference dataset have a lower cyclomatic complexity. This indicates the method has a high number of decisions and it can make the logic difficult to understand and test.
We recommend that you simplify this method or break it into multiple methods. For example, consider extracting the code block on lines 149-155 into a separate method.
|
||
// preminor will bump the version up to the next minor release, and immediately | ||
// down to pre-release. premajor and prepatch work the same way. | ||
public SemVer inc(String release, String identifier) { |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
The cyclomatic complexity of this method is 18. By comparison, 99% of the methods in the CodeGuru reference dataset have a lower cyclomatic complexity. This indicates the method has a high number of decisions and it can make the logic difficult to understand and test. We recommend that you simplify this method or break it into multiple methods.
if (isX(M)) { | ||
ret = ""; | ||
} else if (isX(m)) { | ||
ret = String.format(">=%s.0.0 <%s.0.0-0", M, Integer.parseInt(M) + 1); |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
This code uses '%s' to format int: '+' expression. This is a potential locale-sensitive handling issue. It might cause errors in the handling and processing of the statement at line: 327. Consider formatting this data with '%d' instead.
.collect(Collectors.joining(" ")); | ||
} | ||
|
||
private static String replaceXRange(String s) { |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
The cyclomatic complexity of this method is 19. By comparison, 99% of the methods in the CodeGuru reference dataset have a lower cyclomatic complexity. This indicates the method has a high number of decisions and it can make the logic difficult to understand and test. We recommend that you simplify this method or break it into multiple methods.
.collect(Collectors.joining(" ")); | ||
} | ||
|
||
static String replaceCaret(String c) { |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
The cyclomatic complexity of this method is 11. By comparison, 98% of the methods in the CodeGuru reference dataset have a lower cyclomatic complexity. This indicates the method has a high number of decisions and it can make the logic difficult to understand and test. We recommend that you simplify this method or break it into multiple methods.
return Integer.compare(a, b); | ||
} | ||
|
||
public static int compareIdentifiers(String a, String b) { |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
The cyclomatic complexity of this method is 11. By comparison, 98% of the methods in the CodeGuru reference dataset have a lower cyclomatic complexity. This indicates the method has a high number of decisions and it can make the logic difficult to understand and test. We recommend that you simplify this method or break it into multiple methods.
} | ||
|
||
protected String format() { | ||
String v = String.format("%s.%s.%s", this.major, this.minor, this.patch); |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
This code uses '%s' to format int: patch, major, minor. This is a potential locale-sensitive handling issue. It might cause errors in the handling and processing of the statement at line: 102. Consider formatting this data with '%d' instead.
if (isX(tM)) { | ||
to = ""; | ||
} else if (isX(tm)) { | ||
to = String.format("<%s.0.0-0", Integer.parseInt(tM) + 1); |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
This code uses '%s' to format int: '+' expression. This is a potential locale-sensitive handling issue. It might cause errors in the handling and processing of the statement at line: 434. Consider formatting this data with '%d' instead.
|
||
ret = String.format("%s%s.%s.%s%s", gtlt, M, m, p, pr); | ||
} else if (xm) { | ||
ret = String.format(">=%s.0.0%s <%s.0.0-0", M, pr, Integer.parseInt(M) + 1); |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
This code uses '%s' to format int: '+' expression. This is a potential locale-sensitive handling issue. It might cause errors in the handling and processing of the statement at line: 292. Consider formatting this data with '%d' instead.
return a.compareTo(b) <= 0; | ||
} | ||
|
||
public boolean intersects (Comparator comp) { |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
The cyclomatic complexity of this method is 22. By comparison, 99% of the methods in the CodeGuru reference dataset have a lower cyclomatic complexity. This indicates the method has a high number of decisions and it can make the logic difficult to understand and test.
We recommend that you simplify this method or break it into multiple methods. For example, consider extracting the code block on lines 125-135 into a separate method.
Requirement versionRequirement; | ||
Range versionRequirement; |
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.
Are Requirements and Ranges equivalent?
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.
Yes, that's the semver term for what the other library called a requirement.
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.
I'd recommend downloading this branch and then writing test code to play around with it and check whatever you think is needed. Just a code review with this volume of code is unlikely to find anything.
Translated from npm/node-semver@cb1ca1d. Translated SemVer, Range, Comparator, and their associated tests.
Translated from
npm/node-semver@cb1ca1d.
Translated SemVer, Range, Comparator, and their associated tests.
Issue #, if available:
Description of changes:
Translation of node-semver in order to fix deficiencies in the
com.vdurmont.semver4j
library.Bumped component common library version to 2.2.0-SNAPSHOT.
Why is this change necessary:
How was this change tested:
Ported tests from node-semver which are passing. Existing component recipe tests either passed or were minorly updated to pass.
Any additional information or context required to review the change:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.