-
Notifications
You must be signed in to change notification settings - Fork 473
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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.
…attributes that github adds by default)
@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) |
There was a problem hiding this 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; | ||
|
There was a problem hiding this comment.
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. | ||
# ---------------------------------------------------------------------------- | ||
|
There was a problem hiding this comment.
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 ---------------------------------------------------------------------------- | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
URL?
A quick search yielded https://github.com/bdemers/maven-wrapper superseded by https://github.com/takari/maven-wrapper which itself mentions:
|
Note that maven 3.7.x and 3.8.x seem to have been abandonned in favor of maven 4.0.x |
Is there any plan to carry on this work? |
Note that 3.8.x was released after all and 4.0.x is still not available. |
No description provided.