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 2020/01/09 16:51:32 UTC
[GitHub] [maven-verifier-plugin] KroArtem opened a new pull request #1: Use
new version of site and thoughts about new release
KroArtem opened a new pull request #1: Use new version of site and thoughts about new release
URL: https://github.com/apache/maven-verifier-plugin/pull/1
[description is a wip]
- Cleanup code to align with java8 features
- Mark VerificationResultPrinter as a @FunctionalInterface
- Use java8
- Update dependencies
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [maven-verifier-plugin] slachiewicz commented on a change in pull
request #1: Use new version of site and thoughts about new release
Posted by GitBox <gi...@apache.org>.
slachiewicz commented on a change in pull request #1: Use new version of site and thoughts about new release
URL: https://github.com/apache/maven-verifier-plugin/pull/1#discussion_r365546471
##########
File path: src/main/java/org/apache/maven/plugins/verifier/VerifierMojo.java
##########
@@ -123,17 +122,11 @@ private VerificationResult verify()
{
VerificationResult results = new VerificationResult();
- Reader reader = null;
- try
+ try ( Reader reader = new FileReader( this.verificationFile ) )
Review comment:
good catch - @KroArtem could you create Jira and pr to fix that?
maybe this can be useful https://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html#newBufferedReader(java.nio.file.Path,%20java.nio.charset.Charset) ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [maven-verifier-plugin] elharo commented on a change in pull
request #1: Use new version of site and thoughts about new release
Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #1: Use new version of site and thoughts about new release
URL: https://github.com/apache/maven-verifier-plugin/pull/1#discussion_r365217088
##########
File path: pom.xml
##########
@@ -77,19 +77,20 @@ under the License.
<dependency>
<groupId>org.apache.maven.plugin-tools</groupId>
<artifactId>maven-plugin-annotations</artifactId>
+ <version>3.6.0</version>
Review comment:
If there wasn't a version here before you probably don't want it now. I'd guess it's supplied by thedependencyManagement section of a parent or some such thing.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [maven-verifier-plugin] KroArtem commented on issue #1: Use new
version of site and thoughts about new release
Posted by GitBox <gi...@apache.org>.
KroArtem commented on issue #1: Use new version of site and thoughts about new release
URL: https://github.com/apache/maven-verifier-plugin/pull/1#issuecomment-572928794
Let's wait for developer community replies then.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [maven-verifier-plugin] slachiewicz commented on issue #1: Use new
version of site and thoughts about new release
Posted by GitBox <gi...@apache.org>.
slachiewicz commented on issue #1: Use new version of site and thoughts about new release
URL: https://github.com/apache/maven-verifier-plugin/pull/1#issuecomment-573101934
thx for contribution. I've created https://issues.apache.org/jira/browse/MVERIFIER-36 for that and selected only changes related to move to Java 7 and code cleanup.
Other changes look not so critical for this small plugin so let not try to upgrade to Java 8.
I've also checked where we use that plugin ... and there is no big demand for it. We use it only in the integration suite and that's good reason to release version 3.0.0 and drop Maven 2 compatibility. Even if we then retie plugin.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [maven-verifier-plugin] KroArtem commented on issue #1: Use new
version of site and thoughts about new release
Posted by GitBox <gi...@apache.org>.
KroArtem commented on issue #1: Use new version of site and thoughts about new release
URL: https://github.com/apache/maven-verifier-plugin/pull/1#issuecomment-572708882
Yes, I know the changes are altogether in one PR but it's a sample PR. I've already sent a message on dev@maven.apache.org regarding this plugin. To summarize:
1) Previous version was released in 2015
2) Previous version has old site https://maven.apache.org/plugins/maven-verifier-plugin/
3) Java version is set to 1.7. Is it indeed reasonable when current maven requires 1.8?
4) According to mvnrepository, it's used only in two artifacts, is it worth supporting? https://mvnrepository.com/artifact/org.apache.maven.plugins/maven-verifier-plugin/usages
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [maven-verifier-plugin] elharo commented on a change in pull
request #1: Use new version of site and thoughts about new release
Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #1: Use new version of site and thoughts about new release
URL: https://github.com/apache/maven-verifier-plugin/pull/1#discussion_r365216069
##########
File path: src/main/java/org/apache/maven/plugins/verifier/VerifierMojo.java
##########
@@ -123,17 +122,11 @@ private VerificationResult verify()
{
VerificationResult results = new VerificationResult();
- Reader reader = null;
- try
+ try ( Reader reader = new FileReader( this.verificationFile ) )
Review comment:
not changed in this PR, but this is likely a bug. There's no specified character set
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [maven-verifier-plugin] michael-o commented on issue #1: Use new
version of site and thoughts about new release
Posted by GitBox <gi...@apache.org>.
michael-o commented on issue #1: Use new version of site and thoughts about new release
URL: https://github.com/apache/maven-verifier-plugin/pull/1#issuecomment-572736409
Hard to tell, the dependent ones haven't been updates for years -- nor they cared for this plugin to be updated. I'd rather discontinue this plugin. We have `verify.bsh` or `verify.groovy`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [maven-verifier-plugin] elharo commented on a change in pull
request #1: Use new version of site and thoughts about new release
Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #1: Use new version of site and thoughts about new release
URL: https://github.com/apache/maven-verifier-plugin/pull/1#discussion_r365545873
##########
File path: src/main/java/org/apache/maven/plugins/verifier/VerifierMojo.java
##########
@@ -123,17 +122,11 @@ private VerificationResult verify()
{
VerificationResult results = new VerificationResult();
- Reader reader = null;
- try
+ try ( Reader reader = new FileReader( this.verificationFile ) )
Review comment:
Yes, this is why code shouldn't use FileReader. It's a bad API.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [maven-verifier-plugin] asfgit closed pull request #1: Use new
version of site and thoughts about new release
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1: Use new version of site and thoughts about new release
URL: https://github.com/apache/maven-verifier-plugin/pull/1
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [maven-verifier-plugin] slachiewicz commented on issue #1: Use new
version of site and thoughts about new release
Posted by GitBox <gi...@apache.org>.
slachiewicz commented on issue #1: Use new version of site and thoughts about new release
URL: https://github.com/apache/maven-verifier-plugin/pull/1#issuecomment-573362348
No real benefits thanks to the Java 8 upgrade for this small plugin.
And if we update only this plugin, it will be useful only for the basic Maven project (where we do not use it) and until we upgrade other plugins to a higher version of Java - we can not use it.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [maven-verifier-plugin] KroArtem commented on issue #1: Use new
version of site and thoughts about new release
Posted by GitBox <gi...@apache.org>.
KroArtem commented on issue #1: Use new version of site and thoughts about new release
URL: https://github.com/apache/maven-verifier-plugin/pull/1#issuecomment-573358937
@slachiewicz , thanks! Is there any reason to leave Java 7 version?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [maven-verifier-plugin] KroArtem commented on a change in pull
request #1: Use new version of site and thoughts about new release
Posted by GitBox <gi...@apache.org>.
KroArtem commented on a change in pull request #1: Use new version of site and thoughts about new release
URL: https://github.com/apache/maven-verifier-plugin/pull/1#discussion_r365544608
##########
File path: src/main/java/org/apache/maven/plugins/verifier/VerifierMojo.java
##########
@@ -123,17 +122,11 @@ private VerificationResult verify()
{
VerificationResult results = new VerificationResult();
- Reader reader = null;
- try
+ try ( Reader reader = new FileReader( this.verificationFile ) )
Review comment:
Indeed. There are no open issues about it, though, and according to this answer there is no possibility to set encoding for FileReader until Java 11. https://stackoverflow.com/questions/696626/java-filereader-encoding-issue
(another way still exists, though)
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [maven-verifier-plugin] KroArtem commented on a change in pull
request #1: Use new version of site and thoughts about new release
Posted by GitBox <gi...@apache.org>.
KroArtem commented on a change in pull request #1: Use new version of site and thoughts about new release
URL: https://github.com/apache/maven-verifier-plugin/pull/1#discussion_r365544464
##########
File path: pom.xml
##########
@@ -77,19 +77,20 @@ under the License.
<dependency>
<groupId>org.apache.maven.plugin-tools</groupId>
<artifactId>maven-plugin-annotations</artifactId>
+ <version>3.6.0</version>
Review comment:
Well, I didn't find out any information about 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
With regards,
Apache Git Services
[GitHub] [maven-verifier-plugin] slachiewicz commented on issue #1: Use new
version of site and thoughts about new release
Posted by GitBox <gi...@apache.org>.
slachiewicz commented on issue #1: Use new version of site and thoughts about new release
URL: https://github.com/apache/maven-verifier-plugin/pull/1#issuecomment-573110199
The build was successful - code merged to master. Thx. Let's continue the discussion about the release on the mailing list.
----------------------------------------------------------------
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
With regards,
Apache Git Services