You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/08/26 06:24:17 UTC

[GitHub] [commons-collections] XenoAmess opened a new pull request #178: solve animal sniffer problem that stop build

XenoAmess opened a new pull request #178:
URL: https://github.com/apache/commons-collections/pull/178


   Hi.
   I noticed recent builds cannot pass build on jdk 11+ (actually, not very recent)
   And I tracked it to find ther be a Math.floorMod(long,int) function newly added in 11, and which is not  on jdk8, on jdk8 the codes will invoke Math.floorMod(long,long) .
   That is why animal sniffer will fail the build on 11+.
   As it is just a test file, I've hided it by force casting to long.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess commented on a change in pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
XenoAmess commented on a change in pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#discussion_r477165459



##########
File path: .travis.yml
##########
@@ -29,10 +29,23 @@ jdk:
   - openjdk-ea
 
 script:
-  - mvn -V --no-transfer-progress
   # japicmp requires a package
-  - mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V;
+    mvn -V jar:jar japicmp:cmp -P japicmp;
+    else
+    mvn -V --no-transfer-progress;
+    mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress;
+    fi;
 
 after_success:
   # jacoco will run in the main script. Include the profile to submit to coveralls.
-  - mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco;
+    else
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress;
+    fi;

Review comment:
       @kinow 
   Hi.
   > Couldn't we just drop --no-transfer-progress?
   
   I personally have no experience in using `--no-transfer-progress`, thus I cannot answer this question.
   Sorry for that.
   I suggest if you want to delete it, find the people who add it and get the reason why they add it first.
   I just assume, it is added with a reason...
   




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] coveralls edited a comment on pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-680685964


   
   [![Coverage Status](https://coveralls.io/builds/33011616/badge)](https://coveralls.io/builds/33011616)
   
   Coverage remained the same at 90.127% when pulling **379e4b5c841d88e33e23e648b538f89b375130c0 on XenoAmess:solve_animal_sniffer_problem_that_stop_build** into **cec45f826fad9e0eeccfcb1805300e75c4dfe5d2 on apache:master**.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess commented on a change in pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
XenoAmess commented on a change in pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#discussion_r477174947



##########
File path: .travis.yml
##########
@@ -29,10 +29,23 @@ jdk:
   - openjdk-ea
 
 script:
-  - mvn -V --no-transfer-progress
   # japicmp requires a package
-  - mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V;
+    mvn -V jar:jar japicmp:cmp -P japicmp;
+    else
+    mvn -V --no-transfer-progress;
+    mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress;
+    fi;
 
 after_success:
   # jacoco will run in the main script. Include the profile to submit to coveralls.
-  - mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco;
+    else
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress;
+    fi;

Review comment:
       @kinow 
   Seems:
   only jdk8 on ppc64le(which is not used in commons-lang's ci) have that trouble.
   And jdk8 on AMD64 works fine.
   see: https://travis-ci.org/github/apache/commons-collections/builds/721243529




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] kinow commented on a change in pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#discussion_r477170980



##########
File path: .travis.yml
##########
@@ -29,10 +29,23 @@ jdk:
   - openjdk-ea
 
 script:
-  - mvn -V --no-transfer-progress
   # japicmp requires a package
-  - mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V;
+    mvn -V jar:jar japicmp:cmp -P japicmp;
+    else
+    mvn -V --no-transfer-progress;
+    mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress;
+    fi;
 
 after_success:
   # jacoco will run in the main script. Include the profile to submit to coveralls.
-  - mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco;
+    else
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress;
+    fi;

Review comment:
       No problem @XenoAmess . I had to google that too. I also didn't know about that option, but looking at the docs I think that only reduces the log lines.
   
   Will chase the commit history, and ping the mailing list if necessary. I noticed [lang] has that option too, though it doesn't seem to differentiate JVM version? https://github.com/apache/commons-lang/blob/master/.github/workflows/maven.yml
   
   




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess commented on a change in pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
XenoAmess commented on a change in pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#discussion_r477193630



##########
File path: .travis.yml
##########
@@ -29,10 +29,23 @@ jdk:
   - openjdk-ea
 
 script:
-  - mvn -V --no-transfer-progress
   # japicmp requires a package
-  - mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V;
+    mvn -V jar:jar japicmp:cmp -P japicmp;
+    else
+    mvn -V --no-transfer-progress;
+    mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress;
+    fi;
 
 after_success:
   # jacoco will run in the main script. Include the profile to submit to coveralls.
-  - mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco;
+    else
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress;
+    fi;

Review comment:
       @kinow
   I cannot find how to get ARCH variable from travis-ci's official docs.
   So I'm just, trying every String that seems possible lol.
   But seems:
   deleting `--no-transfer-progress` will NOT get the build from success to fail.
   
   So I'm personally +1 on deleting `--no-transfer-progress`




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] coveralls edited a comment on pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-680685964


   
   [![Coverage Status](https://coveralls.io/builds/33011045/badge)](https://coveralls.io/builds/33011045)
   
   Coverage remained the same at 90.127% when pulling **45ed4b88fc01a568b954297c0055de6757c40cc9 on XenoAmess:solve_animal_sniffer_problem_that_stop_build** into **cec45f826fad9e0eeccfcb1805300e75c4dfe5d2 on apache:master**.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess commented on a change in pull request #178: solve animal sniffer problem that fails build on 11+

Posted by GitBox <gi...@apache.org>.
XenoAmess commented on a change in pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#discussion_r477080104



##########
File path: src/main/java/org/apache/commons/collections4/iterators/ListIteratorWrapper.java
##########
@@ -204,7 +204,7 @@ public int previousIndex() {
 
     /**
      * Removes the last element that was returned by {@link #next()} or {@link #previous()} from the underlying collection.
-     * This call can only be made once per call to {@code next} or {@code previous} and only if {@link #add()} was not called in between.
+     * This call can only be made once per call to {@code next} or {@code previous} and only if {@link #add(Object)} was not called in between.

Review comment:
       @kinow no.
   that is yet another error that stop the build.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] coveralls edited a comment on pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-680685964


   
   [![Coverage Status](https://coveralls.io/builds/33008167/badge)](https://coveralls.io/builds/33008167)
   
   Coverage decreased (-0.03%) to 90.098% when pulling **c302eea1f85de55de980c633a5fa185d4cdafb6d on XenoAmess:solve_animal_sniffer_problem_that_stop_build** into **cec45f826fad9e0eeccfcb1805300e75c4dfe5d2 on apache:master**.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess commented on pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
XenoAmess commented on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-680708899


   the second commit solved a javadoc error that will fail the build on 11+.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess edited a comment on pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
XenoAmess edited a comment on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-680730361


   @kinow Hi.
   It can pass ci now.
   Suggest merge as fast as possible...
   Thanks.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] kinow commented on pull request #178: solve animal sniffer problem that fails build on 11+

Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-680692001


   @XenoAmess I had noticed the error, and tracked down to that function, but wasn't sure what had changed. Let's leave CI run/build it now, but looks like you've fixed this annoying issue! Thanks a lot!!! :champagne: 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess edited a comment on pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
XenoAmess edited a comment on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-681398369


   @kinow 
   Hi.
   After search in docs / try by self, I can find NO way to get ARCH variable from travis-ci.
   So we MIGHT either use if condition to JDK_NAME (see c302eea above),
   or delete `--no-transfer-progress`.  (see 379e4b5 above).
   Both can pass ci, and acceptable.
   
   Personally I slightly prefer deleting `--no-transfer-progress`, because seems it is added not to solve an build error, but to slightly reduce travis-ci's pressure.
   
   So please choose one for the final choice, and then I will rebase / sqruash.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] kinow commented on a change in pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#discussion_r477145595



##########
File path: .travis.yml
##########
@@ -29,10 +29,23 @@ jdk:
   - openjdk-ea
 
 script:
-  - mvn -V --no-transfer-progress
   # japicmp requires a package
-  - mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V;
+    mvn -V jar:jar japicmp:cmp -P japicmp;
+    else
+    mvn -V --no-transfer-progress;
+    mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress;
+    fi;
 
 after_success:
   # jacoco will run in the main script. Include the profile to submit to coveralls.
-  - mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco;
+    else
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress;
+    fi;

Review comment:
       Couldn't we just drop `--no-transfer-progress`? Having this in the CI scripts could cause some confusion in the next vote thread (we would either have to include instructions for people building on JDK8, or ask/hope them to check the CI scripts).
   
   If dropping it fixes, with the consequence that the logs may be a bit longer, then I think that should be OK. WDYT?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess commented on a change in pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
XenoAmess commented on a change in pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#discussion_r477174947



##########
File path: .travis.yml
##########
@@ -29,10 +29,23 @@ jdk:
   - openjdk-ea
 
 script:
-  - mvn -V --no-transfer-progress
   # japicmp requires a package
-  - mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V;
+    mvn -V jar:jar japicmp:cmp -P japicmp;
+    else
+    mvn -V --no-transfer-progress;
+    mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress;
+    fi;
 
 after_success:
   # jacoco will run in the main script. Include the profile to submit to coveralls.
-  - mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco;
+    else
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress;
+    fi;

Review comment:
       @kinow 
   Seems:
   only jdk8 on ppc64le(which is not used in commons-lang's ci) have that trouble.
   And jdk8 on AMD64 works fine.
   




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess commented on a change in pull request #178: solve animal sniffer problem that fails build on 11+

Posted by GitBox <gi...@apache.org>.
XenoAmess commented on a change in pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#discussion_r477080104



##########
File path: src/main/java/org/apache/commons/collections4/iterators/ListIteratorWrapper.java
##########
@@ -204,7 +204,7 @@ public int previousIndex() {
 
     /**
      * Removes the last element that was returned by {@link #next()} or {@link #previous()} from the underlying collection.
-     * This call can only be made once per call to {@code next} or {@code previous} and only if {@link #add()} was not called in between.
+     * This call can only be made once per call to {@code next} or {@code previous} and only if {@link #add(Object)} was not called in between.

Review comment:
       @kinow
   no.
   that is yet another error that stop the build.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] coveralls edited a comment on pull request #178: solve animal sniffer problem that fails build on 11+

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-680685964


   
   [![Coverage Status](https://coveralls.io/builds/33006850/badge)](https://coveralls.io/builds/33006850)
   
   Coverage remained the same at 90.127% when pulling **b95bd7a9be8ec929818e3712f7bcea278e239874 on XenoAmess:solve_animal_sniffer_problem_that_stop_build** into **cec45f826fad9e0eeccfcb1805300e75c4dfe5d2 on apache:master**.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess commented on a change in pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
XenoAmess commented on a change in pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#discussion_r477174947



##########
File path: .travis.yml
##########
@@ -29,10 +29,23 @@ jdk:
   - openjdk-ea
 
 script:
-  - mvn -V --no-transfer-progress
   # japicmp requires a package
-  - mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V;
+    mvn -V jar:jar japicmp:cmp -P japicmp;
+    else
+    mvn -V --no-transfer-progress;
+    mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress;
+    fi;
 
 after_success:
   # jacoco will run in the main script. Include the profile to submit to coveralls.
-  - mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco;
+    else
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress;
+    fi;

Review comment:
       @kinow 
   Seems:
   only jdk8 on ppc64le (which is not used in commons-lang's ci) have that trouble.
   And jdk8 on AMD64 works fine.
   see: https://travis-ci.org/github/apache/commons-collections/builds/721243529
   
   But I have no experience with ppc64le either.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess commented on pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
XenoAmess commented on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-680709554


   the third commit solved a problem that --no-transfer-progress is not usable on openjdk8.
   so I added a condition select in travis.yml.
   will see if it can work correctly.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess commented on pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
XenoAmess commented on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-680730361


   @kinow Hi.
   It can pass ci now.
   Suggest merge as fast as possible...


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] coveralls edited a comment on pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-680685964


   
   [![Coverage Status](https://coveralls.io/builds/33011402/badge)](https://coveralls.io/builds/33011402)
   
   Coverage remained the same at 90.127% when pulling **379e4b5c841d88e33e23e648b538f89b375130c0 on XenoAmess:solve_animal_sniffer_problem_that_stop_build** into **cec45f826fad9e0eeccfcb1805300e75c4dfe5d2 on apache:master**.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess commented on a change in pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
XenoAmess commented on a change in pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#discussion_r477179031



##########
File path: .travis.yml
##########
@@ -29,10 +29,23 @@ jdk:
   - openjdk-ea
 
 script:
-  - mvn -V --no-transfer-progress
   # japicmp requires a package
-  - mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V;
+    mvn -V jar:jar japicmp:cmp -P japicmp;
+    else
+    mvn -V --no-transfer-progress;
+    mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress;
+    fi;
 
 after_success:
   # jacoco will run in the main script. Include the profile to submit to coveralls.
-  - mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco;
+    else
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress;
+    fi;

Review comment:
       @kinow 
   I'm trying to change the if condition from jdk-version to os-name.
   If it can work, then the problem is from ppc64le but not jdk8.
   (A little curious : why we need a test environment on ppc64le? Is there some historical reason?)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] kinow closed pull request #178: [COLLECTIONS-766] fix ci build (or to say, fix everything that wrongly blocked the ci build.)

Posted by GitBox <gi...@apache.org>.
kinow closed pull request #178:
URL: https://github.com/apache/commons-collections/pull/178


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess commented on a change in pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
XenoAmess commented on a change in pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#discussion_r477193630



##########
File path: .travis.yml
##########
@@ -29,10 +29,23 @@ jdk:
   - openjdk-ea
 
 script:
-  - mvn -V --no-transfer-progress
   # japicmp requires a package
-  - mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V;
+    mvn -V jar:jar japicmp:cmp -P japicmp;
+    else
+    mvn -V --no-transfer-progress;
+    mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress;
+    fi;
 
 after_success:
   # jacoco will run in the main script. Include the profile to submit to coveralls.
-  - mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco;
+    else
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress;
+    fi;

Review comment:
       @kinow
   I cannot find how to get ARCH variable from travis-ci's official docs.
   So I'm just, trying every String that seems possible lol.
   But seems:
   deleting `--no-transfer-progress` will not get the build from success to fail.
   
   So I'm personally +1 on deleting `--no-transfer-progress`




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] kinow commented on a change in pull request #178: solve animal sniffer problem that fails build on 11+

Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#discussion_r477070887



##########
File path: src/main/java/org/apache/commons/collections4/iterators/ListIteratorWrapper.java
##########
@@ -204,7 +204,7 @@ public int previousIndex() {
 
     /**
      * Removes the last element that was returned by {@link #next()} or {@link #previous()} from the underlying collection.
-     * This call can only be made once per call to {@code next} or {@code previous} and only if {@link #add()} was not called in between.
+     * This call can only be made once per call to {@code next} or {@code previous} and only if {@link #add(Object)} was not called in between.

Review comment:
       Was this necessary to solve the animal sniffer problems too?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess commented on pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
XenoAmess commented on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-681398369


   @kinow 
   Hi.
   After search in docs / try by self, I can find NO way to get ARCH variable from travis-ci.
   So we MIGHT either use if condition to JDK_NAME (see c302eea above),
   or delete `--no-transfer-progress`.  (see 379e4b5 above).
   Both can pass ci, and acceptable.
   
   Personally I slightly lean on deleting `--no-transfer-progress`, because seems it is added not to solve an build error, but to slightly reduce travis-ci's pressure.
   
   So please choose one for the final choice, and then I will rebase / sqruash.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess edited a comment on pull request #178: [COLLECTIONS-766] fix ci build (or to say, fix everything that wrongly blocked the ci build.)

Posted by GitBox <gi...@apache.org>.
XenoAmess edited a comment on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-681398369


   @kinow 
   Hi.
   After search in docs / try by self, I can find NO way to get ARCH variable from travis-ci.
   So we MIGHT either use if condition to JDK_NAME (see c302eea above),
   or delete `--no-transfer-progress`.  (see 379e4b5 above).
   Both can pass ci, and acceptable.
   
   Personally I slightly prefer deleting `--no-transfer-progress`, because seems it is added not to solve an build error, but to slightly reduce travis-ci's pressure.
   
   Aynway, please choose one for the final choice, and then I will rebase / sqruash.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] coveralls edited a comment on pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-680685964


   
   [![Coverage Status](https://coveralls.io/builds/33011616/badge)](https://coveralls.io/builds/33011616)
   
   Coverage remained the same at 90.127% when pulling **379e4b5c841d88e33e23e648b538f89b375130c0 on XenoAmess:solve_animal_sniffer_problem_that_stop_build** into **cec45f826fad9e0eeccfcb1805300e75c4dfe5d2 on apache:master**.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] coveralls commented on pull request #178: solve animal sniffer problem that fails build on 11+

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-680685964


   
   [![Coverage Status](https://coveralls.io/builds/33006619/badge)](https://coveralls.io/builds/33006619)
   
   Coverage remained the same at 90.127% when pulling **df2a58cefe27d9f996a14dd786a21f8d0185b2ff on XenoAmess:solve_animal_sniffer_problem_that_stop_build** into **cec45f826fad9e0eeccfcb1805300e75c4dfe5d2 on apache:master**.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess edited a comment on pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
XenoAmess edited a comment on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-680709554


   the third commit solved a problem that `--no-transfer-progress` is not usable on openjdk8.
   so I added a condition select in travis.yml.
   will see if it can work correctly.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess edited a comment on pull request #178: [COLLECTIONS-766] fix ci build (or to say, fix everything that wrongly blocked the ci build.)

Posted by GitBox <gi...@apache.org>.
XenoAmess edited a comment on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-680709554


   the third and later commit solved a problem that `--no-transfer-progress` is not usable on openjdk8 at ppc64le .
   so I added a condition select in travis.yml.
   will see if it can work correctly.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess edited a comment on pull request #178: [COLLECTIONS-766] fix ci build (or to say, fix everything that wrongly blocked the ci build.)

Posted by GitBox <gi...@apache.org>.
XenoAmess edited a comment on pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#issuecomment-680709554


   the third and later commit solved a problem that `--no-transfer-progress` is not usable on openjdk8.
   so I added a condition select in travis.yml.
   will see if it can work correctly.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-collections] XenoAmess commented on a change in pull request #178: fix ci builds

Posted by GitBox <gi...@apache.org>.
XenoAmess commented on a change in pull request #178:
URL: https://github.com/apache/commons-collections/pull/178#discussion_r477165459



##########
File path: .travis.yml
##########
@@ -29,10 +29,23 @@ jdk:
   - openjdk-ea
 
 script:
-  - mvn -V --no-transfer-progress
   # japicmp requires a package
-  - mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V;
+    mvn -V jar:jar japicmp:cmp -P japicmp;
+    else
+    mvn -V --no-transfer-progress;
+    mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress;
+    fi;
 
 after_success:
   # jacoco will run in the main script. Include the profile to submit to coveralls.
-  - mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco;
+    else
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress;
+    fi;

Review comment:
       @kinow 
   Hi.
   > Couldn't we just drop --no-transfer-progress?
   I personally have no experience in using `--no-transfer-progress`, thus I cannot answer this question.
   Sorry for that.
   I suggest if you want to delete it, find people who add it and get the reason why they add it first.
   I just assume, it is added with a reason...
   

##########
File path: .travis.yml
##########
@@ -29,10 +29,23 @@ jdk:
   - openjdk-ea
 
 script:
-  - mvn -V --no-transfer-progress
   # japicmp requires a package
-  - mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V;
+    mvn -V jar:jar japicmp:cmp -P japicmp;
+    else
+    mvn -V --no-transfer-progress;
+    mvn -V jar:jar japicmp:cmp -P japicmp --no-transfer-progress;
+    fi;
 
 after_success:
   # jacoco will run in the main script. Include the profile to submit to coveralls.
-  - mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress
+  - |
+    if [ "$TRAVIS_JDK_VERSION" = "openjdk8" ];
+    then
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco;
+    else
+    mvn -V jacoco:report coveralls:report -Ptravis-jacoco --no-transfer-progress;
+    fi;

Review comment:
       @kinow 
   Hi.
   > Couldn't we just drop --no-transfer-progress?
   
   I personally have no experience in using `--no-transfer-progress`, thus I cannot answer this question.
   Sorry for that.
   I suggest if you want to delete it, find people who add it and get the reason why they add it first.
   I just assume, it is added with a reason...
   




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org