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/05/31 20:45:04 UTC
[GitHub] [maven-resolver] michael-o opened a new pull request #52: [MRESOLVER-56] Support SHA-256 and SHA-512 as checksums
michael-o opened a new pull request #52:
URL: https://github.com/apache/maven-resolver/pull/52
----------------------------------------------------------------
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-resolver] asfgit closed pull request #52: [MRESOLVER-56] Support SHA-256 and SHA-512 as checksums
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #52:
URL: https://github.com/apache/maven-resolver/pull/52
----------------------------------------------------------------
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-resolver] mthmulders commented on a change in pull request #52: [MRESOLVER-56] Support SHA-256 and SHA-512 as checksums
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #52:
URL: https://github.com/apache/maven-resolver/pull/52#discussion_r435002545
##########
File path: maven-resolver-connector-basic/src/test/java/org/eclipse/aether/connector/basic/ChecksumCalculatorTest.java
##########
@@ -8,9 +8,9 @@
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
- *
+ *
Review comment:
I've seen other pull requests against Maven where changing whitespace was not OK, so I figured that it wouldn't be OK here 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] [maven-resolver] mthmulders commented on a change in pull request #52: [MRESOLVER-56] Support SHA-256 and SHA-512 as checksums
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #52:
URL: https://github.com/apache/maven-resolver/pull/52#discussion_r433064648
##########
File path: maven-resolver-connector-basic/src/test/java/org/eclipse/aether/connector/basic/ChecksumCalculatorTest.java
##########
@@ -8,9 +8,9 @@
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
- *
+ *
Review comment:
Unrelated change. By the way, there's a lot of these (mainly in the header comments, but also in a few other places).
----------------------------------------------------------------
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-resolver] michael-o commented on a change in pull request #52: [MRESOLVER-56] Support SHA-256 and SHA-512 as checksums
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #52:
URL: https://github.com/apache/maven-resolver/pull/52#discussion_r435522550
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/SimpleDigest.java
##########
@@ -29,21 +29,21 @@
class SimpleDigest
{
Review comment:
Will add, but also note that this class is package private in internal impl.
----------------------------------------------------------------
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-resolver] mthmulders commented on a change in pull request #52: [MRESOLVER-56] Support SHA-256 and SHA-512 as checksums
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #52:
URL: https://github.com/apache/maven-resolver/pull/52#discussion_r433065216
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/SimpleDigest.java
##########
@@ -37,18 +37,32 @@
{
try
{
- digest = MessageDigest.getInstance( "SHA-1" );
+ digest = MessageDigest.getInstance( "SHA-512" );
Review comment:
Can't we replace this fourfold nested try/catch with a loop over an ordered collection?
----------------------------------------------------------------
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-resolver] rfscholte commented on a change in pull request #52: [MRESOLVER-56] Support SHA-256 and SHA-512 as checksums
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #52:
URL: https://github.com/apache/maven-resolver/pull/52#discussion_r433091079
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/SimpleDigest.java
##########
@@ -37,18 +37,32 @@
{
try
{
- digest = MessageDigest.getInstance( "SHA-1" );
+ digest = MessageDigest.getInstance( "SHA-512" );
Review comment:
I spotted the same codesmell: nested try-catch blocks. Should be able to rewrite that. with a single try-catch level
----------------------------------------------------------------
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-resolver] rfscholte commented on a change in pull request #52: [MRESOLVER-56] Support SHA-256 and SHA-512 as checksums
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #52:
URL: https://github.com/apache/maven-resolver/pull/52#discussion_r435518611
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/SimpleDigest.java
##########
@@ -29,21 +29,21 @@
class SimpleDigest
{
Review comment:
I know this hasn't changed, but `SimpleDigest` doesn't reflect it functionality. With the extra options it makes sense to improve the javadoc saying it searches for the strongest available digest.
----------------------------------------------------------------
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-resolver] michael-o commented on a change in pull request #52: [MRESOLVER-56] Support SHA-256 and SHA-512 as checksums
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #52:
URL: https://github.com/apache/maven-resolver/pull/52#discussion_r435525328
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/SimpleDigest.java
##########
@@ -29,21 +29,21 @@
class SimpleDigest
{
Review comment:
Done.
----------------------------------------------------------------
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-resolver] michael-o commented on a change in pull request #52: [MRESOLVER-56] Support SHA-256 and SHA-512 as checksums
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #52:
URL: https://github.com/apache/maven-resolver/pull/52#discussion_r433114599
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/SimpleDigest.java
##########
@@ -37,18 +37,32 @@
{
try
{
- digest = MessageDigest.getInstance( "SHA-1" );
+ digest = MessageDigest.getInstance( "SHA-512" );
Review comment:
Will do and catch the last exception.
----------------------------------------------------------------
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-resolver] michael-o commented on a change in pull request #52: [MRESOLVER-56] Support SHA-256 and SHA-512 as checksums
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #52:
URL: https://github.com/apache/maven-resolver/pull/52#discussion_r433114433
##########
File path: maven-resolver-connector-basic/src/test/java/org/eclipse/aether/connector/basic/ChecksumCalculatorTest.java
##########
@@ -8,9 +8,9 @@
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
- *
+ *
Review comment:
That's because my Eclipse autotrims trailing whitespace. I can undo that of course.
----------------------------------------------------------------
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-resolver] michael-o commented on a change in pull request #52: [MRESOLVER-56] Support SHA-256 and SHA-512 as checksums
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #52:
URL: https://github.com/apache/maven-resolver/pull/52#discussion_r434830074
##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/SimpleDigest.java
##########
@@ -37,18 +37,32 @@
{
try
{
- digest = MessageDigest.getInstance( "SHA-1" );
+ digest = MessageDigest.getInstance( "SHA-512" );
Review comment:
Done!
----------------------------------------------------------------
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-resolver] michael-o commented on a change in pull request #52: [MRESOLVER-56] Support SHA-256 and SHA-512 as checksums
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #52:
URL: https://github.com/apache/maven-resolver/pull/52#discussion_r435158282
##########
File path: maven-resolver-connector-basic/src/test/java/org/eclipse/aether/connector/basic/ChecksumCalculatorTest.java
##########
@@ -8,9 +8,9 @@
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
- *
+ *
Review comment:
I wouldn't object if you provide a PR where trailing whitescape is trimmed. What we dislike is reformatting unrelated code spots.
----------------------------------------------------------------
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-resolver] mthmulders commented on a change in pull request #52: [MRESOLVER-56] Support SHA-256 and SHA-512 as checksums
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #52:
URL: https://github.com/apache/maven-resolver/pull/52#discussion_r433064648
##########
File path: maven-resolver-connector-basic/src/test/java/org/eclipse/aether/connector/basic/ChecksumCalculatorTest.java
##########
@@ -8,9 +8,9 @@
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
- *
+ *
Review comment:
Unrelated change
----------------------------------------------------------------
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-resolver] mthmulders commented on a change in pull request #52: [MRESOLVER-56] Support SHA-256 and SHA-512 as checksums
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #52:
URL: https://github.com/apache/maven-resolver/pull/52#discussion_r433064648
##########
File path: maven-resolver-connector-basic/src/test/java/org/eclipse/aether/connector/basic/ChecksumCalculatorTest.java
##########
@@ -8,9 +8,9 @@
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
- *
+ *
Review comment:
Unrelated change. By the way, there's a lot of these (in the header comments).
----------------------------------------------------------------
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