You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by GitBox <gi...@apache.org> on 2020/09/01 15:59:36 UTC

[GitHub] [maven-archiver] elharo opened a new pull request #10: [MSHARED-955] make parseOutputTimestamp static

elharo opened a new pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10


   @hboutemy  @mabrarov


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] mabrarov commented on pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
mabrarov commented on pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10#issuecomment-684968771


   @pzygielo, sorry for a stupid question, but does it make sense if this change will be released with new version of Maven Archiver? Isn't it sufficient to notify users of this library about this incompatible class change in release notes so that they will become aware of it when migrating to the new version of Maven Archiver?


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] elharo commented on pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
elharo commented on pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10#issuecomment-684984178


   I think the issue only appears if a subclass overrides this method.


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] pzygielo commented on pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
pzygielo commented on pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10#issuecomment-685044143


   > @pzygielo can you provide more details on what code would be required to trigger this?
   
   See https://github.com/pzrep/MSHARED-955
   


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] mabrarov commented on pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
mabrarov commented on pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10#issuecomment-763180478


   Is this PR still actual?


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] pzygielo commented on pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
pzygielo commented on pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10#issuecomment-684976887


   > Isn't it sufficient to notify users of this library about this incompatible class change 
   
   Maybe it is sufficient, but one has to be aware of that to prepare notification.


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] mabrarov commented on pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
mabrarov commented on pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10#issuecomment-685043977


   To be honest implementation of `parseOutputTimestamp` looks wrong because it doesn't parse optional time zone. This causes issues with `project.build.outputTimestamp` maven property having values like `2020-05-01T12:12:12Z`. It's a separate issue but may be we could combine changes into a single pull request.


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] mabrarov edited a comment on pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
mabrarov edited a comment on pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10#issuecomment-685043977


   To be honest implementation of `parseOutputTimestamp` looks wrong because it doesn't parse optional time zone. This causes issues with `project.build.outputTimestamp` maven property having values like `2020-05-01T12:12:12Z`. It's a separate issue but may be we could combine changes into a single pull request.
   
   It looks I was wrong. Deleting this comment to not spoil this PR discussion.


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] elharo commented on pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
elharo commented on pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10#issuecomment-685019474


   @pzygielo can you provide more details on what code would be required to trigger this? 


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] pzygielo commented on pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
pzygielo commented on pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10#issuecomment-684965881


   It is `IncompatibleClassChange` (https://github.com/apache/maven-ear-plugin/pull/10#discussion_r481249976).


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] mabrarov removed a comment on pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
mabrarov removed a comment on pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10#issuecomment-685043977


   To be honest implementation of `parseOutputTimestamp` looks wrong because it doesn't parse optional time zone. This causes issues with `project.build.outputTimestamp` maven property having values like `2020-05-01T12:12:12Z`. It's a separate issue but may be we could combine changes into a single pull request.
   
   It looks I was wrong. Deleting this comment to not spoil this PR discussion.


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] elharo commented on pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
elharo commented on pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10#issuecomment-685063812


   Thanks. That's not something I'm worried about happening in practice. Is there a likely diamond dependency problem where the incompatible version gets selected? 


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] pzygielo commented on pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
pzygielo commented on pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10#issuecomment-685128506


   > That's not something I'm worried about happening in practice. 
   
   I know. But I do.
   
   So I've updated my example for the case, where this dependency is overriden in assembly plugin (i.e. archiver is no longer direct dependency of code in my example, but is utilized by plugin).
   
   Per http://maven.apache.org/plugins/maven-assembly-plugin/dependencies.html assembly plugin uses archiver in version 3.5.0 (the latest).
   
   Per https://maven.apache.org/guides/mini/guide-configuring-plugins.html#Using_the_dependencies_Tag:
   > You could configure the dependencies of the Build plugins, commonly to use a more recent dependency 
   > version.
   
   So I do in my example, pretending that 3.5.1-SNAPSHOT is released now and I like other changes in it, and there is no better release of assembly plugin. Example shows that proposed change is not backward compatible.
   
   ---
   I'm only saying, that if there is no other way - it shall be reflected in version of plugin, as per https://semver.org/, which might not be observed here but is kind of standard:
   > increment MAJOR version when you make incompatible API changes,
   
   or **at least** as @mabrarov said - very clearly stated in RN.


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] pzygielo commented on pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
pzygielo commented on pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10#issuecomment-684998151


   > I think the issue only appears if a subclass overrides this method.
   
   If so changed class appears in place of old one - without recompiling client - then `IncompatibleClassChangeError` would be raised.<sup>1</sup> Thus it depends how maven-archiver artifact is used.
   
   1. I imagine overriding maven-jar/ear-plugin dependency to use new release of maven-archiver for example.


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] elharo closed pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
elharo closed pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10


   


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] rfscholte commented on pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
rfscholte commented on pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10#issuecomment-685436731


   For the record: Maven doesn't really understand semver, it is just a hint. If for the same groupId+artifactId a version 1.0 and 3.1 are in the dependency graph, Maven will pick only 1 based on the "nearest wins"-rule. Marking breaking changes with the versions might work for frameworks like spring, but not for libraries like commons-lang. Hence they made the right decision to change both artifactId and the package to prevent collisions.


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] [maven-archiver] michael-o commented on pull request #10: [MSHARED-955] make parseOutputTimestamp static

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #10:
URL: https://github.com/apache/maven-archiver/pull/10#issuecomment-763471268


   Likely, @hboutemy 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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org