Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename checkSignature to alwaysVerifyChecksum #191

Merged

Conversation

kriegaex
Copy link
Contributor

@kriegaex kriegaex commented Jun 8, 2021

Fixes #186, see the naming and documentation discussion there.

First, the plan was to just rename the one parameter and keep the one with the old name around as deprecated. while doing so, I noticed lots of

  • class and package names,
  • method names and parameters,
  • javadocs and comments,

referring to digest-based checksums like SHA-1 and MD5 as "signatures" too, so I took care of globally renaming all of those too. That should make it easier to work with the code base in the future.

BTW, along the way I encountered the problem solved by #190, so maybe you want to merge than one first, if you like to run ITs on JDK 16 against this PR. Otherwise, the merge order does not matter.

Copy link
Contributor

@longtimeago longtimeago left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kriegaex very nice! Thanks for the contribution 👍
Just tiny styling remarks

Fixes maven-download-plugin#186, see the naming and documentation discussion there.

First, the plan was to just rename the one parameter and keep the one
with the old name around as deprecated. while doing so, I noticed lots
of
  - class and package names,
  - method names and parameters,
  - javadocs and comments,
referring to digest-based checksums like SHA-1 and MD5 as "signatures"
too, so I took care of globally renaming all of those too. That should
make it easier to work with the code base in the future.
Copy link
Contributor

@longtimeago longtimeago left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kriegaex thank you! 👍

@longtimeago longtimeago merged commit 25b3917 into maven-download-plugin:master Jun 8, 2021
@kriegaex kriegaex deleted the always-verify-checksum branch June 8, 2021 10:17
kriegaex added a commit to kriegaex/aspectj that referenced this pull request Sep 5, 2021
After my PR maven-download-plugin/maven-download-plugin#191
was merged and maven-download-plugin/maven-download-plugin#186
was closed, use a new release containing the option misnomer fix in
order to make the POM more readable.

Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

After failed wget download, no checksum is compared during the next build
2 participants