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

JAMES-3398 add Maven wrapper (with latest Maven version 3.6.3) #253

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

aasaru
Copy link
Member

@aasaru aasaru commented Oct 14, 2020

No description provided.

Copy link
Contributor

@chibenwa chibenwa left a comment

Choose a reason for hiding this comment

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

I think we should not commit binaries here. As a commiter, I can not ensure the authenticity nor the integrity of it. (I would not commit it to the james project as it is)

Your PR don't explain nor document how to use the wrapper (and why). A bit of ducmuentation in developer documentation could be helpful ;-)

Cheers.

@aasaru
Copy link
Member Author

aasaru commented Oct 14, 2020

@chibenwa , fair points both, I agree with you. I now removed the binary and added separate section to readme.

I also improved the readme a bit. I investigated why IntelliJ IDEA complains about Readme.adoc and fixed some of the problems it pointed out (I couldn't fix titles that contain "+" sign so IntelliJ IDEA will still complain about those)

Copy link
Contributor

@chibenwa chibenwa left a comment

Choose a reason for hiding this comment

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

Sorry for radio silence.

I agree with the proposed changes, thanks for putting that together!

import java.io.*;
import java.nio.channels.*;
import java.util.Properties;

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand this code is copied but can we have a link to the place it was copied from as a comment?

This helps code patternity.

This helps checking divergences from the original version.

# specific language governing permissions and limitations
# under the License.
# ----------------------------------------------------------------------------

Copy link
Contributor

Choose a reason for hiding this comment

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

Idem, I would welcome a URL from where this was copied.

@REM specific language governing permissions and limitations
@REM under the License.
@REM ----------------------------------------------------------------------------

Copy link
Contributor

Choose a reason for hiding this comment

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

URL?

@jeantil
Copy link
Contributor

jeantil commented Dec 29, 2020

A quick search yielded https://github.com/bdemers/maven-wrapper superseded by https://github.com/takari/maven-wrapper which itself mentions:

The project codebase has been accepted to be included in the upstream Apache Maven project itself. Currently the plan is to release the wrapper as a feature of the upcoming Maven 3.7.0 release

@jeantil
Copy link
Contributor

jeantil commented Apr 1, 2021

Note that maven 3.7.x and 3.8.x seem to have been abandonned in favor of maven 4.0.x
There is an alpha snapshot floating around for fearless osx using people : https://maarten.mulders.it/2020/11/whats-new-in-maven-4/ there is also probably a way to use it on linux...

@chibenwa
Copy link
Contributor

Is there any plan to carry on this work?

@jeantil
Copy link
Contributor

jeantil commented Aug 30, 2022

Note that 3.8.x was released after all and 4.0.x is still not available.
in the 3.8.x release, maven wrapper was added as an official maven plugin https://maven.apache.org/wrapper/maven-wrapper-plugin/index.html

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.

3 participants