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

Update mvnw and CI on JDK 21 #80

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

mbien
Copy link
Member

@mbien mbien commented Nov 21, 2024

No description provided.

@mbien mbien requested a review from neilcsmith-net November 21, 2024 16:35
Copy link
Member

@neilcsmith-net neilcsmith-net left a comment

Choose a reason for hiding this comment

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

Thanks. But needs the hash regenerating.

@@ -14,6 +14,6 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip
wrapperUrl=https://repo.maven.apache.org/maven2/org/apache/maven/wrapper/maven-wrapper/3.2.0/maven-wrapper-3.2.0.jar
distributionSha256Sum=83aaf914c785c9faed661f223000a92d1de9553f5c82d3b4362e66d9c031625f
Copy link
Member

Choose a reason for hiding this comment

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

Please don't remove the hash. Will need regenerating, and mvnw might also need patching to pass on macOS - see https://issues.apache.org/jira/browse/MWRAPPER-150

Copy link
Member Author

Choose a reason for hiding this comment

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

this wasn't intended. I simply ran the mvnw updater. Will fix it.

Copy link
Member

@neilcsmith-net neilcsmith-net Nov 21, 2024

Choose a reason for hiding this comment

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

Yes, it's annoying it doesn't handle this. And that it uses sha256 so the hashes aren't published anywhere. I've just got in the habit of patching these files manually. Thanks for looking at this!

Copy link
Member Author

@mbien mbien Nov 21, 2024

Choose a reason for hiding this comment

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

btw do you want to keep the wrapperUrl property too? the wrapper did remove that one also.

Copy link
Member

Choose a reason for hiding this comment

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

No, that's unused IIRC. Should have removed it when I last updated this.

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.

2 participants