Skip to content

Simplify detection of the MAVEN_HOME and make it fully qualified#307

Closed
rostidev wants to merge 1 commit intoapache:masterfrom
rostidev:patch-2
Closed

Simplify detection of the MAVEN_HOME and make it fully qualified#307
rostidev wants to merge 1 commit intoapache:masterfrom
rostidev:patch-2

Conversation

@rostidev
Copy link
Copy Markdown
Contributor

@rostidev rostidev commented Jan 1, 2020

The removed 'stripMHome' recursive function is not needed anymore.

Part of the 'mvn -version' output before this change:
Maven home: D:\develop\apache-maven-3.6.3\bin..

Same part of the 'mvn -version' output after this change:
Maven home: D:\develop\apache-maven-3.6.3
I.e. now it's fully qualified.

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Make sure there is a JIRA issue filed
    for the change (usually before you start working on it). Trivial changes like typos do not
    require a JIRA issue. Your pull request should address just this issue, without
    pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [MNG-XXX] - Fixes bug in ApproximateQuantiles,
    where you replace MNG-XXX with the appropriate JIRA issue. Best practice
    is to use the JIRA issue title in the pull request title and in the first line of the
    commit message.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean verify to make sure basic checks pass. A more thorough check will
    be performed on your pull request automatically.
  • You have run the Core IT successfully.

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

@slachiewicz
Copy link
Copy Markdown
Member

Thank you for contribution - could you also create Jira here: https://issues.apache.org/jira/browse/MNG ?

@rostidev
Copy link
Copy Markdown
Contributor Author

rostidev commented Jan 1, 2020

Created a Jira issue about this improvement:
https://issues.apache.org/jira/browse/MNG-6837

@slachiewicz
Copy link
Copy Markdown
Member

Copy link
Copy Markdown
Member

@slachiewicz slachiewicz left a comment

Choose a reason for hiding this comment

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

looks good to me, anyone else can check also?

@rostidev
Copy link
Copy Markdown
Contributor Author

rostidev commented Jan 2, 2020

I want to change this line in my commit:
set "MAVEN_HOME=%MAVEN_HOME:\bin\=%"

into this one:
set "MAVEN_HOME=%MAVEN_HOME:~0,-5%"

The reason: it's safer because theoretically the "\bin\" substring could appear more than once but we need to remove only the last one.

I will amend my commit and push force it.

The removed 'stripMHome' recursive function is not needed anymore.

Part of the 'mvn -version' output before this change:
Maven home: D:\develop\apache-maven-3.6.3\bin\..

Same part of the 'mvn -version' output after this change:
Maven home: D:\develop\apache-maven-3.6.3
I.e. now it's fully qualified.
@slachiewicz
Copy link
Copy Markdown
Member

Quick question - what form where we use " should be better?
set "MAVEN_HOME=xxxx" or set MAVEN_HOME="xxxx"

@rostidev
Copy link
Copy Markdown
Contributor Author

rostidev commented Jan 3, 2020

I just kept the original style of using " in set commands. But now I found that this is not only a matter of style but a mandatory requirement. Using set A="value with spaces" inserts the double quotes into the variable value and we don't want this. Using set "A=value with spaces" doesn't insert the double quotes into the variable value and this is good.

https://stackoverflow.com/questions/535975/dealing-with-quotes-in-windows-batch-scripts

Comment thread apache-maven/src/bin/mvn.cmd
asfgit pushed a commit that referenced this pull request Jan 13, 2020
…lified on Windows

The removed 'stripMHome' recursive function is not needed anymore

This closes #307
@michael-o michael-o self-requested a review January 13, 2020 21:11
@asfgit asfgit closed this in 9989e6a Jan 13, 2020
asfgit pushed a commit that referenced this pull request Jun 26, 2021
…lified on Windows

The removed 'stripMHome' recursive function is not needed anymore

This closes #307
@jira-importer
Copy link
Copy Markdown

Resolve #8451

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.

4 participants