You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/01/26 10:11:21 UTC
[GitHub] [maven-surefire] kriegaex opened a new pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
kriegaex opened a new pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332
Cast to Buffer to avoid java.lang.NoSuchMethodError due to JDK API
breakage.
This was fixed in a similar way in apache/maven-wagon@92c0d2a.
See mongodb/mongo-java-driver@21c91bd for details.
----------------------------------------------------------------
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] [maven-surefire] kriegaex edited a comment on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex edited a comment on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767947574
> Maybe you want to contribute instead of micro-managing my edits? 😉
After having checked all your code review suggestions plus all the other places I changed which you did not even mention, my assumption was confirmed that collaboratively working on this would have saved time. If you had just checked out the PR and tried to flip types in your IDE, you would have noticed. I think that code reviews outside of an IDE which actually compiles are not particularly helpful in this case where it is about changing types in order to avoid casting. So - no offence meant - this was a waste of time for both of us and I want to re-invite you to piggy-back on my PR and just commit whatever improvements you have for it. Before you even write "could you edit this like that?", you would have done it already with no more time spent but without me spending time on checking, commenting back, you reading the comments again.
We could
* avoid micro-management,
* save time,
* get better code faster,
* avoid some of the 7 wastes of lean production, such as hand-offs, stretching the cycle time by scattering the touch times across a longer than necessary time frame. You are a busy developer and doing OSS in your spare time, I assume. Even if someone pays you for it, you do not want to create waste.
Sorry for lobbying here and making this a general discussion, but I always do that when contributing to a project for the first time in order to find out if it would be attractive for me to contribute again in the future. I am strictly against the policy in so many OSS projects that PRs get micro-managed in many iterations up to the point in which the contributor has shaped the code into something the reviewer would have written. Better to accept an imperfect, but valuable PR and help polishing it by yourself. Then the chances are much higher that the code ends up thhe way you like it as a maintainer. Also, the contributor can learn from the edits. Win-win.
----------------------------------------------------------------
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] [maven-surefire] kriegaex edited a comment on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex edited a comment on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767476849
Maybe you want to contribute instead of micro-managing my edits? 😉
Just add commits to this PR to your heart's content. I am an agile coach in my daytime job, not a software developer. So I am all open for collaborative software development because this is what I am teaching to my coachee dev teams.
----------------------------------------------------------------
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] [maven-surefire] kriegaex commented on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex commented on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-768682455
@Tibor17, thanks for the Slack invitation. Please contact me through one of the channels listed on my [StackOverflow profile](https://stackoverflow.com/users/1082681/kriegaex?tab=profile) (scroll to the bottom of my profile text), e.g. Gitter or Telegram. There I can tell you my e-mail address.
----------------------------------------------------------------
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] [maven-surefire] kriegaex commented on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex commented on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767476849
Maybe you want to contribute instead of micro-managing my edits? 😉
----------------------------------------------------------------
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] [maven-surefire] kriegaex commented on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex commented on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767947574
> Maybe you want to contribute instead of micro-managing my edits? 😉
After having checked all your code review suggestions plus all the other places I changed which you did not even mention, my assumption that collaboratively working on this would have saved time. If you had just checked out the PR and tried to flip types in your IDE, you would have noticed. I think that code reviews outside of an IDE which actually compiles are not particularly helpful in this case where it is about changing types in order to avoid casting. So - no offence meant - this was a waste of time for both of us and I want to re-invite you to piggy-back on my PR and just commit whatever improvements you have for it. Before you even write "could you edit this like that?", you would have done it already with no more time spent but without me spending time on checking, commenting back, you reading the comments again.
We could
* avoid micro-management,
* save time,
* get better code faster,
* avoid some of the 7 wastes of lean production, such as hand-offs, stretching the cycle time by scattering the touch times across a longer than necessary time frame. You are a busy developer and doing OSS in your spare time, I assume. Even if someone pays you for it, you do not want to create waste.
Sorry for lobbying here and making this a general discussion, but I always do that when contributing to a project for the first time in order to find out if it would be attractive for me to contribute again in the future. I am strictly against the policy in so many OSS projects that PRs get micro-managed in many iterations up to the point in which the contributor has shaped the code into something the reviewer would have written. Better to accept an imperfect, but valuable PR and help polishing it by yourself. Then the chances are much higher that the code ends up thhe way you like it as a maintainer. Also, the contributor can learn from the edits. Win-win.
----------------------------------------------------------------
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] [maven-surefire] kriegaex edited a comment on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex edited a comment on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767947574
> Maybe you want to contribute instead of micro-managing my edits? 😉
After having checked all your code review suggestions plus all the other places I changed which you did not even mention, my assumption was confirmed that collaboratively working on this would have saved time. If you had just checked out the PR and tried to flip types in your IDE, you would have noticed. I think that code reviews outside of an IDE which actually compiles are not particularly helpful in this case where it is about changing types in order to avoid casting. So - no offence meant - this was a waste of time for both of us and I want to re-invite you to piggy-back on my PR and just commit whatever improvements you have for it. Before you could even write "could you edit this like that?", you would have done it already with no more time spent but without me spending time on checking, commenting back, you reading the comments again.
We could
* avoid micro-management,
* save time,
* get better code faster,
* avoid some of the [7 wastes of lean production](https://agilecoachdiaries.wordpress.com/2017/01/19/lean-software-development-7-wastes-of-software-development/), such as hand-offs, stretching the cycle time by scattering the touch times across a longer than necessary time frame, in turn requiring more context changes for both of us, trying to remember where we left off last time. You are a busy developer and doing OSS in your spare time, I assume. Even if someone pays you for it, you do not want to create waste.
Sorry for lobbying here and making this a general discussion, but I always do that when contributing to a project for the first time in order to find out if it would be attractive for me to contribute again in the future. I am strictly against the policy in so many OSS projects that PRs get micro-managed in many iterations up to the point in which the contributor, not unlike a remote-controlled puppet, has shaped the code into something resembling what the reviewer would have written by herself. Better to accept an imperfect, but valuable PR and help polishing it by yourself. Then chances are much higher that the code ends up the way you like it as a maintainer. Also, the contributor can learn from the edits. Win-win.
----------------------------------------------------------------
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] [maven-surefire] Tibor17 commented on a change in pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#discussion_r564434105
##########
File path: surefire-api/src/main/java/org/apache/maven/surefire/api/stream/AbstractStreamEncoder.java
##########
@@ -112,17 +113,17 @@ public void encodeString( CharsetEncoder encoder, ByteBuffer result, String stri
{
String nonNullString = nonNull( string );
- int counterPosition = result.position();
+ int counterPosition = ( (Buffer) result ).position();
Review comment:
`position` method is also in the Buffer class. Maybe we can directly declare such type.
----------------------------------------------------------------
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] [maven-surefire] Tibor17 commented on a change in pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#discussion_r564432905
##########
File path: surefire-api/src/main/java/org/apache/maven/surefire/api/stream/AbstractStreamDecoder.java
##########
@@ -28,6 +28,7 @@
import java.io.EOFException;
import java.io.File;
import java.io.IOException;
+import java.nio.Buffer;
Review comment:
Can we declare the variables as `Buffer` in this class instead of ByteBuffer?
----------------------------------------------------------------
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] [maven-surefire] kriegaex edited a comment on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex edited a comment on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767947574
> Maybe you want to contribute instead of micro-managing my edits? 😉
After having checked all your code review suggestions plus all the other places I changed which you did not even mention, my assumption was confirmed that collaboratively working on this would have saved time. If you had just checked out the PR and tried to flip types in your IDE, you would have noticed. I think that code reviews outside of an IDE which actually compiles are not particularly helpful in this case where it is about changing types in order to avoid casting. So - no offence meant - this was a waste of time for both of us and I want to re-invite you to piggy-back on my PR and just commit whatever improvements you have for it. Before you could even write "could you edit this like that?", you would have done it already with no more time spent but without me spending time on checking, commenting back, you reading the comments again.
We could
* avoid micro-management,
* save time,
* get better code faster,
* avoid some of the 7 wastes of lean production, such as hand-offs, stretching the cycle time by scattering the touch times across a longer than necessary time frame. You are a busy developer and doing OSS in your spare time, I assume. Even if someone pays you for it, you do not want to create waste.
Sorry for lobbying here and making this a general discussion, but I always do that when contributing to a project for the first time in order to find out if it would be attractive for me to contribute again in the future. I am strictly against the policy in so many OSS projects that PRs get micro-managed in many iterations up to the point in which the contributor has shaped the code into something the reviewer would have written. Better to accept an imperfect, but valuable PR and help polishing it by yourself. Then the chances are much higher that the code ends up thhe way you like it as a maintainer. Also, the contributor can learn from the edits. Win-win.
----------------------------------------------------------------
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] [maven-surefire] kriegaex commented on a change in pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex commented on a change in pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#discussion_r564941015
##########
File path: maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/extensions/StreamFeederTest.java
##########
@@ -90,9 +91,9 @@ public void shouldEncodeCommandToStream() throws Exception
public Object answer( InvocationOnMock invocation ) throws IOException
{
ByteBuffer bb = invocation.getArgument( 0 );
Review comment:
`bb.array()` two lines further down would then return `Object` instead of `byte[]` which would mean a cast in that place instead. Is that preferable in your opinion?
----------------------------------------------------------------
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] [maven-surefire] kriegaex commented on a change in pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex commented on a change in pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#discussion_r564955600
##########
File path: maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporterTest.java
##########
@@ -380,7 +381,7 @@ public void testSyncOnDeferredFile() throws Exception
{
Utf8RecodingDeferredFileOutputStream out = new Utf8RecodingDeferredFileOutputStream( "test" );
ByteBuffer cache = ByteBuffer.wrap( new byte[] {1, 2, 3} );
- cache.position( 3 );
+ ( (Buffer) cache ).position( 3 );
Review comment:
I checked all tests and also all application code by manually flipping local `ByteBuffer` variables to `Buffer`, and this is the only place where this is actually possible. In all other places one of the following stops me from easily changing `ByteBuffer` into `Buffer` types:
* Other `ByteBuffer` methods are being used, requiring casts in the reverse direction to `ByteBuffer`.
* The variable is declared as a method parameter, requiring signature changes, possibly recursively.
* The variable is used in method calls requiring a cast in reverse direction to `ByteBuffer`.
----------------------------------------------------------------
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] [maven-surefire] kriegaex commented on a change in pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex commented on a change in pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#discussion_r564956338
##########
File path: surefire-api/src/main/java/org/apache/maven/surefire/api/stream/AbstractStreamDecoder.java
##########
@@ -28,6 +28,7 @@
import java.io.EOFException;
import java.io.File;
import java.io.IOException;
+import java.nio.Buffer;
Review comment:
No, we cannot, see https://github.com/apache/maven-surefire/pull/332#discussion_r564955600
----------------------------------------------------------------
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] [maven-surefire] kriegaex commented on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex commented on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767453228
Looks like CI failure was caused by a network problem:
```text
Caused by: java.net.UnknownHostException: repo.maven.apache.org
at java.net.Inet6AddressImpl.lookupAllHostAddr (Native Method)
at java.net.InetAddress$2.lookupAllHostAddr (InetAddress.java:929)
at java.net.InetAddress.getAddressesFromNameService (InetAddress.java:1324)
at java.net.InetAddress.getAllByName0 (InetAddress.java:1277)
at java.net.InetAddress.getAllByName (InetAddress.java:1193)
at java.net.InetAddress.getAllByName (InetAddress.java:1127)
at org.apache.maven.wagon.providers.http.httpclient.impl.conn.SystemDefaultDnsResolver.resolve (SystemDefaultDnsResolver.java:45)
```
The same build was successful on my local workstation a few minutes before the CI run.
----------------------------------------------------------------
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] [maven-surefire] Tibor17 edited a comment on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
Tibor17 edited a comment on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-774051665
@kriegaex
The Apache changed the politcy and so only people with `apache.org` can be invited via e-mail.
----------------------------------------------------------------
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] [maven-surefire] Tibor17 commented on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767465083
@kriegaex I did rerun of the build. The networ is bad sometimes.
----------------------------------------------------------------
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] [maven-surefire] kriegaex edited a comment on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex edited a comment on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767947574
> Maybe you want to contribute instead of micro-managing my edits? 😉
After having checked all your code review suggestions plus all the other places I changed which you did not even mention, my assumption was confirmed that collaboratively working on this would have saved time. If you had just checked out the PR and tried to flip types in your IDE, you would have noticed. I think that code reviews outside of an IDE which actually compiles are not particularly helpful in this case where it is about changing types in order to avoid casting. So - no offence meant - this was a waste of time for both of us and I want to re-invite you to piggy-back on my PR and just commit whatever improvements you have for it. Before you could even write "could you edit this like that?", you would have done it already with no more time spent but without me spending time on checking, commenting back, you reading the comments again.
We could
* avoid micro-management,
* save time,
* get better code faster,
* avoid some of the [7 wastes of lean production](https://agilecoachdiaries.wordpress.com/2017/01/19/lean-software-development-7-wastes-of-software-development/), such as hand-offs, stretching the cycle time by scattering the touch times across a longer than necessary time frame, in turn requiring more context changes for both of us, trying to remember where we left off last time. You are a busy developer and doing OSS in your spare time, I assume. Even if someone pays you for it, you do not want to create waste.
Sorry for lobbying here and making this a general discussion, but I always do that when contributing to a project for the first time in order to find out if it would be attractive for me to contribute again in the future. I am strictly against the policy in so many OSS projects that PRs get micro-managed in many iterations up to the point in which the contributor, not unlike a remote-controlled puppet, has shaped the code into something the reviewer would have written. Better to accept an imperfect, but valuable PR and help polishing it by yourself. Then the chances are much higher that the code ends up the way you like it as a maintainer. Also, the contributor can learn from the edits. Win-win.
----------------------------------------------------------------
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] [maven-surefire] Tibor17 commented on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-768563554
@kriegaex
Thx for contributing!
----------------------------------------------------------------
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] [maven-surefire] kriegaex commented on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex commented on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767927878
Is the Travis CI build really still necessary?
----------------------------------------------------------------
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] [maven-surefire] Tibor17 merged pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
Tibor17 merged pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332
----------------------------------------------------------------
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] [maven-surefire] Tibor17 commented on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767947085
We can move on if the fix is complete.
----------------------------------------------------------------
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] [maven-surefire] kriegaex edited a comment on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex edited a comment on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767947574
> Maybe you want to contribute instead of micro-managing my edits? 😉
After having checked all your code review suggestions plus all the other places I changed which you did not even mention, my assumption was confirmed that collaboratively working on this would have saved time. If you had just checked out the PR and tried to flip types in your IDE, you would have noticed. I think that code reviews outside of an IDE which actually compiles are not particularly helpful in this case where it is about changing types in order to avoid casting. So - no offence meant - this was a waste of time for both of us and I want to re-invite you to piggy-back on my PR and just commit whatever improvements you have for it. Before you could even write "could you edit this like that?", you would have done it already with no more time spent but without me spending time on checking, commenting back, you reading the comments again.
We could
* avoid micro-management,
* save time,
* get better code faster,
* avoid some of the 7 wastes of lean production, such as hand-offs, stretching the cycle time by scattering the touch times across a longer than necessary time frame, in turn requiring more context changes for both of us, trying to remember where we left of last time. You are a busy developer and doing OSS in your spare time, I assume. Even if someone pays you for it, you do not want to create waste.
Sorry for lobbying here and making this a general discussion, but I always do that when contributing to a project for the first time in order to find out if it would be attractive for me to contribute again in the future. I am strictly against the policy in so many OSS projects that PRs get micro-managed in many iterations up to the point in which the contributor has shaped the code into something the reviewer would have written. Better to accept an imperfect, but valuable PR and help polishing it by yourself. Then the chances are much higher that the code ends up thhe way you like it as a maintainer. Also, the contributor can learn from the edits. Win-win.
----------------------------------------------------------------
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] [maven-surefire] kriegaex edited a comment on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex edited a comment on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767947574
> Maybe you want to contribute instead of micro-managing my edits? 😉
After having checked all your code review suggestions plus all the other places I changed which you did not even mention, my assumption was confirmed that collaboratively working on this would have saved time. If you had just checked out the PR and tried to flip types in your IDE, you would have noticed. I think that code reviews outside of an IDE which actually compiles are not particularly helpful in this case where it is about changing types in order to avoid casting. So - no offence meant - this was a waste of time for both of us and I want to re-invite you to piggy-back on my PR and just commit whatever improvements you have for it. Before you could even write "could you edit this like that?", you would have done it already with no more time spent but without me spending time on checking, commenting back, you reading the comments again.
We could
* avoid micro-management,
* save time,
* get better code faster,
* avoid some of the [7 wastes of lean production](https://agilecoachdiaries.wordpress.com/2017/01/19/lean-software-development-7-wastes-of-software-development/), such as hand-offs, stretching the cycle time by scattering the touch times across a longer than necessary time frame, in turn requiring more context changes for both of us, trying to remember where we left off last time. You are a busy developer and doing OSS in your spare time, I assume. Even if someone pays you for it, you do not want to create waste.
Sorry for lobbying here and making this a general discussion, but I always do that when contributing to a project for the first time in order to find out if it would be attractive for me to contribute again in the future. I am strictly against the policy in so many OSS projects that PRs get micro-managed in many iterations up to the point in which the contributor has shaped the code into something the reviewer would have written. Better to accept an imperfect, but valuable PR and help polishing it by yourself. Then the chances are much higher that the code ends up the way you like it as a maintainer. Also, the contributor can learn from the edits. Win-win.
----------------------------------------------------------------
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] [maven-surefire] kriegaex commented on a change in pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex commented on a change in pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#discussion_r564942022
##########
File path: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/Utf8RecodingDeferredFileOutputStream.java
##########
@@ -93,14 +94,14 @@ public synchronized void write( String output, boolean newLine )
}
else
{
- cache.flip();
+ ( (Buffer) cache ).flip();
Review comment:
Again, at some point `array()` is called, returning `Object` instead of `byte[]`. Furthermore, `put()` is called twice, a method unavailable in `Buffer`.
----------------------------------------------------------------
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] [maven-surefire] Tibor17 commented on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-774051665
@kriegaex
The Apache changed the politcy and so only people with `apache.org` can be invited.
----------------------------------------------------------------
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] [maven-surefire] Tibor17 commented on a change in pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#discussion_r564430136
##########
File path: maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/extensions/StreamFeederTest.java
##########
@@ -90,9 +91,9 @@ public void shouldEncodeCommandToStream() throws Exception
public Object answer( InvocationOnMock invocation ) throws IOException
{
ByteBuffer bb = invocation.getArgument( 0 );
Review comment:
I mean to declare the type as Buffer, see my proposal `Buffer bb = invocation.getArgument( 0 );`
----------------------------------------------------------------
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] [maven-surefire] Tibor17 commented on a change in pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#discussion_r564430829
##########
File path: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/Utf8RecodingDeferredFileOutputStream.java
##########
@@ -93,14 +94,14 @@ public synchronized void write( String output, boolean newLine )
}
else
{
- cache.flip();
+ ( (Buffer) cache ).flip();
Review comment:
Can we declare the `cache` as `Buffer` here as well?
----------------------------------------------------------------
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] [maven-surefire] kriegaex edited a comment on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex edited a comment on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767476849
Maybe you want to contribute instead of micro-managing my edits? 😉
Just add commits to this PR to your heart's content. I would not mind at all, considering myself your junior in this project anyway. I am an agile coach in my daytime job, not a professional software developer (I was 20 years ago). So I am all open for collaborative software development because this is what I am teaching to my coachee dev teams.
----------------------------------------------------------------
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] [maven-surefire] kriegaex commented on a change in pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex commented on a change in pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#discussion_r564965487
##########
File path: maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporterTest.java
##########
@@ -380,7 +381,7 @@ public void testSyncOnDeferredFile() throws Exception
{
Utf8RecodingDeferredFileOutputStream out = new Utf8RecodingDeferredFileOutputStream( "test" );
ByteBuffer cache = ByteBuffer.wrap( new byte[] {1, 2, 3} );
- cache.position( 3 );
+ ( (Buffer) cache ).position( 3 );
Review comment:
Sorry for kicking off all CI builds for this little change. Thanks for the review.
----------------------------------------------------------------
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] [maven-surefire] Tibor17 commented on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-768472337
@kriegaex
Are you willing to be invited to our Apache Slack channel? Maybe just like a coacher or a friend and contributor.
I know that technically it is possible via an email but i do not see your email.
----------------------------------------------------------------
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] [maven-surefire] kriegaex edited a comment on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex edited a comment on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767453228
Looks like the "CI for Windows 1" failure was caused by a network problem:
```text
Caused by: java.net.UnknownHostException: repo.maven.apache.org
at java.net.Inet6AddressImpl.lookupAllHostAddr (Native Method)
at java.net.InetAddress$2.lookupAllHostAddr (InetAddress.java:929)
at java.net.InetAddress.getAddressesFromNameService (InetAddress.java:1324)
at java.net.InetAddress.getAllByName0 (InetAddress.java:1277)
at java.net.InetAddress.getAllByName (InetAddress.java:1193)
at java.net.InetAddress.getAllByName (InetAddress.java:1127)
at org.apache.maven.wagon.providers.http.httpclient.impl.conn.SystemDefaultDnsResolver.resolve (SystemDefaultDnsResolver.java:45)
```
The same build was successful on my local workstation a few minutes before the CI run.
----------------------------------------------------------------
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] [maven-surefire] kriegaex commented on a change in pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex commented on a change in pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#discussion_r564957111
##########
File path: surefire-api/src/main/java/org/apache/maven/surefire/api/stream/AbstractStreamEncoder.java
##########
@@ -112,17 +113,17 @@ public void encodeString( CharsetEncoder encoder, ByteBuffer result, String stri
{
String nonNullString = nonNull( string );
- int counterPosition = result.position();
+ int counterPosition = ( (Buffer) result ).position();
Review comment:
No, see https://github.com/apache/maven-surefire/pull/332#discussion_r564955600. Furthermore, the call `encoder.encode( wrap( nonNullString ), result, true );` requires `result` to be a `ByteBuffer` again.
----------------------------------------------------------------
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] [maven-surefire] Tibor17 commented on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767945402
I talked about the Travis CI with our Infra team. They have migrated `travis-ci.org` to `travis-ci.com`. Let's leave the Travis alone for a while but it will come back in some form ;-)
----------------------------------------------------------------
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] [maven-surefire] Tibor17 commented on a change in pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#discussion_r564429679
##########
File path: maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/extensions/StreamFeederTest.java
##########
@@ -90,9 +91,9 @@ public void shouldEncodeCommandToStream() throws Exception
public Object answer( InvocationOnMock invocation ) throws IOException
{
ByteBuffer bb = invocation.getArgument( 0 );
Review comment:
Maybe it would be nicer to declare the type here and then we do not have to cast `bb`.
----------------------------------------------------------------
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] [maven-surefire] Tibor17 commented on a change in pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#discussion_r564431601
##########
File path: maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporterTest.java
##########
@@ -380,7 +381,7 @@ public void testSyncOnDeferredFile() throws Exception
{
Utf8RecodingDeferredFileOutputStream out = new Utf8RecodingDeferredFileOutputStream( "test" );
ByteBuffer cache = ByteBuffer.wrap( new byte[] {1, 2, 3} );
- cache.position( 3 );
+ ( (Buffer) cache ).position( 3 );
Review comment:
Can we declare the `cache` as `Buffer` in the tests as well?
----------------------------------------------------------------
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] [maven-surefire] kriegaex edited a comment on pull request #332: [SUREFIRE-1882] Fix failures when compiled on Java 9+ and run on Java 8
Posted by GitBox <gi...@apache.org>.
kriegaex edited a comment on pull request #332:
URL: https://github.com/apache/maven-surefire/pull/332#issuecomment-767947574
> Maybe you want to contribute instead of micro-managing my edits? 😉
After having checked all your code review suggestions plus all the other places I changed which you did not even mention, my assumption was confirmed that collaboratively working on this would have saved time. If you had just checked out the PR and tried to flip types in your IDE, you would have noticed. I think that code reviews outside of an IDE which actually compiles are not particularly helpful in this case where it is about changing types in order to avoid casting. So - no offence meant - this was a waste of time for both of us and I want to re-invite you to piggy-back on my PR and just commit whatever improvements you have for it. Before you could even write "could you edit this like that?", you would have done it already with no more time spent but without me spending time on checking, commenting back, you reading the comments again.
We could
* avoid micro-management,
* save time,
* get better code faster,
* avoid some of the 7 wastes of lean production, such as hand-offs, stretching the cycle time by scattering the touch times across a longer than necessary time frame, in turn requiring more context changes for both of us, trying to remember where we left of last time. You are a busy developer and doing OSS in your spare time, I assume. Even if someone pays you for it, you do not want to create waste.
Sorry for lobbying here and making this a general discussion, but I always do that when contributing to a project for the first time in order to find out if it would be attractive for me to contribute again in the future. I am strictly against the policy in so many OSS projects that PRs get micro-managed in many iterations up to the point in which the contributor has shaped the code into something the reviewer would have written. Better to accept an imperfect, but valuable PR and help polishing it by yourself. Then the chances are much higher that the code ends up the way you like it as a maintainer. Also, the contributor can learn from the edits. Win-win.
----------------------------------------------------------------
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