You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/01/11 22:39:35 UTC
[GitHub] [solr] HoustonPutman opened a new pull request #515: SOLR-15910: Fork semver4j library to fix semver bugs.
HoustonPutman opened a new pull request #515:
URL: https://github.com/apache/solr/pull/515
https://issues.apache.org/jira/browse/SOLR-SOLR-15910
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on pull request #515: SOLR-15910: Fork semver4j library to fix semver bugs.
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #515:
URL: https://github.com/apache/solr/pull/515#issuecomment-1012287224
Completely agree with both of you that not forking would be the best path forward. It's hard company-policy-wise for me to move forward with either of those options, but I would appreciate either of you taking it up!
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #515: SOLR-15910: Fork semver4j library to fix semver bugs.
Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #515:
URL: https://github.com/apache/solr/pull/515#issuecomment-1011075619
Maybe for an adjacent library, just having a fixed version that lives as someone's personal fork in Github would also be okay? Versus bringing in this code to the Solr project itself.
Github makes it too easy to get lots of forks that all end up vaguely unmaintained, and I wish it encouraged a cluster of forks like what we see for semver to somehow come back together to a single maintained approach. Oh wait... _community over code_ ;-)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #515: SOLR-15910: Fork semver4j library to fix semver bugs.
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #515:
URL: https://github.com/apache/solr/pull/515#issuecomment-1010446080
There are already 11 contributors to https://github.com/vdurmont/semver4j. Have you tried contacting them? Perhspa if we submit a PR for https://github.com/vdurmont/semver4j/issues/48 and ping a few of the devs, we can help trigger a new release? Two years old can also mean that it is mature and don't need that many updates...
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #515: SOLR-15910: Fork semver4j library to fix semver bugs.
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #515:
URL: https://github.com/apache/solr/pull/515#issuecomment-1013107663
I'll not veto the code copy, if you clearly mark it and open, say, a blocker JIRA for 10.0 or something to replace it with something that is maintained...
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #515: SOLR-15910: Fork semver4j library to fix semver bugs.
Posted by GitBox <gi...@apache.org>.
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 vdurmont@gmail.com, 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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] madrob commented on pull request #515: SOLR-15910: Fork semver4j library to fix semver bugs.
Posted by GitBox <gi...@apache.org>.
madrob commented on pull request #515:
URL: https://github.com/apache/solr/pull/515#issuecomment-1010439994
This is kind of a pain, but I’d like to see the commits split into a clean fork and then our changes separately. Also, I don’t think we repackage the files, see org.noggit
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org