janhoy commented on a change in pull request #515: URL: https://github.com/apache/solr/pull/515#discussion_r782588226
########## File path: solr/NOTICE.txt ########## @@ -95,6 +95,10 @@ This project includes the Malihu Custom Scrollbar Plugin Copyright (c) Manos Malihutsakis, http://manos.malihu.gr/ License: MIT https://github.com/malihu/malihu-custom-scrollbar-plugin/blob/master/LICENSE.txt +This project includes code forked from the semver4j +Copyright (c) 2015-present Vincent DURMONT [email protected], https://github.com/vdurmont/semver4j +License: MIT https://github.com/vdurmont/semver4j/blob/master/LICENSE.md Review comment: Actually I think the right location is `LICENSE.txt` for these unless they are apache licenses that require NOTICE. But we have been doing it wrong for a long time. ########## File path: solr/core/src/java/org/apache/solr/util/SolrVersion.java ########## @@ -95,30 +85,31 @@ public boolean lessThanOrEqualTo(SolrVersion other) { */ public boolean satisfies(String semVerExpression) { try { - return ExpressionParser.newInstance().parse(semVerExpression).interpret(version); - } catch (ParseException parseException) { - throw new InvalidSemVerExpressionException(); + Semver semNPM = new Semver(toString(), Semver.SemverType.NPM); Review comment: I also tested this lib last week, but found that the epression support was limited compared to the one we use. You have to choose dialect, so you get `>1.2.2` but not `>1.2` or `1.x`. But as long as we document the syntax we support in package mgr we should be ok ########## File path: solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java ########## @@ -193,7 +193,7 @@ public static String resolve(String str, Map<String, String> defaults, Map<Strin * Compares two versions v1 and v2. Returns negative if v1 isLessThan v2, positive if v1 isGreaterThan v2 and 0 if equal. */ public static int compareVersions(String v1, String v2) { - return Version.valueOf(v1).compareTo(Version.valueOf(v2)); + return (new Semver(v1, Semver.SemverType.NPM)).compareTo(new Semver(v2, Semver.SemverType.NPM)); Review comment: I thought I had delegated this to `SolrVersion`. You could do ```java return SolrVersion.valueOf(v1).compareTo(SolrVersion.valueOf(v2)) ``` Then the choice of NPM or watever will reside inside SolrVersion class. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
