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

Add Maven Wrapper #185

Merged
merged 1 commit into from
Sep 6, 2024
Merged

Add Maven Wrapper #185

merged 1 commit into from
Sep 6, 2024

Conversation

vorburger
Copy link
Member

Fixes #175.

@J-N-K and @sebthom OK for you? Plz Merge if LGTY; thanks!

PS: Ignore the revert commit on this PR (due to this), I just couldn't figure out how to get rid of it, here.

@sebthom
Copy link
Member

sebthom commented Aug 30, 2024

just out of interest how is the process in upgrading the wrapper or the maven version? is there e. g. a command that can upgrade both to the latest minor version? If so we could add a custom ci job to create PRs accordingly, since dependabot does not yet support upgrading maven wrapper dependabot/dependabot-core#485 but there is https://github.com/marketplace/actions/update-gradle-wrapper-action

I haven't used the wrapper myself so far.


@sebthom
Copy link
Member

sebthom commented Aug 30, 2024

I saw the build failed because of a commit signing is missing. does the "signed-of-by" commit message appendix actually have any value? It is not cryptographic anyway. I wouldn't mind to remove it. None of the eclipse projects I am contributing to has it.

@J-N-K
Copy link
Member

J-N-K commented Aug 30, 2024

It's a kind of "CLA per commit", see https://wiki.linuxfoundation.org/dco. IIRC eclipse projects require signing a CLA prior to the first contribution.

@J-N-K
Copy link
Member

J-N-K commented Aug 30, 2024

@vorburger Currently the diff shows "0 changed files".

J-N-K
J-N-K previously approved these changes Aug 30, 2024
Copy link
Member

@sebthom sebthom left a comment

Choose a reason for hiding this comment

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

This commit needs to be removed.

@sebthom
Copy link
Member

sebthom commented Sep 4, 2024

I rebased the branch which resolves the issue of the diff showing "0 changed files".

@vorburger Can we merge it without commit sign-off? Btw. the contribution guidelines don't say anything about a requirement for commit signing. Maybe they should be extended and maybe a pull request template should be created that names that requirement.

@sebthom sebthom self-requested a review September 4, 2024 09:08
@vorburger
Copy link
Member Author

I rebased the branch which resolves the issue of the diff showing "0 changed files".

Thank You!

just out of interest how is the process in upgrading the wrapper or the maven version? is there e. g. a command that can upgrade both to the latest minor version? If so we could add a custom ci job to create PRs accordingly, since dependabot does not yet support upgrading maven wrapper dependabot/dependabot-core#485 but there is https://github.com/marketplace/actions/update-gradle-wrapper-action

FYI somehow (to my own surprise!) Dependabot actually seem to proposed version bumps upgrades for the Maven Wrapper... see https://github.com/enola-dev/enola/pull/869/files for an example I've just run into!

does the "signed-of-by" commit message appendix actually have any value? It is not cryptographic anyway. I wouldn't mind to remove it. None of the eclipse projects I am contributing to has it.

Re. ^^^ this is why:

It's a kind of "CLA per commit", see https://wiki.linuxfoundation.org/dco. IIRC eclipse projects require signing a CLA prior to the first contribution.

Maybe they should be extended and maybe a pull request template should be created that names that requirement.

Good idea! Do you want to do that? I do think this could avoid hypothetical problems - if this was ever used by more people.

Can we merge it without commit sign-off?

I'll fix it.

Signed-off-by: Michael Vorburger <mike@vorburger.ch>
@vorburger vorburger merged commit 05ea682 into main Sep 6, 2024
3 checks passed
@sebthom sebthom deleted the maven-wrapper branch September 6, 2024 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Adopt Maven Wrapper
3 participants