You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2020/12/02 15:32:40 UTC

[GitHub] [maven-release] K-J-Henken opened a new pull request #62: [MRELEASE-899] Feature/lineseparator

K-J-Henken opened a new pull request #62:
URL: https://github.com/apache/maven-release/pull/62


   https://issues.apache.org/jira/browse/MRELEASE-899


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-release] michael-o commented on a change in pull request #62: [MRELEASE-899] Feature/lineseparator

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #62:
URL: https://github.com/apache/maven-release/pull/62#discussion_r808368072



##########
File path: maven-release-manager/src/main/mdo/release-descriptor.mdo
##########
@@ -604,6 +604,16 @@
           </description>
         </field>
 
+        <field>
+          <name>lineSeparator</name>
+          <version>3.0.0+</version>
+          <type>String</type>
+          <description>
+            Specifies the line separator to use in the pom.xml.
+            (https://issues.apache.org/jira/browse/MRELEASE-899)

Review comment:
       Redundant

##########
File path: maven-release-api/src/main/java/org/apache/maven/shared/release/config/ReleaseDescriptor.java
##########
@@ -567,6 +567,15 @@
      */
     String getAutoResolveSnapshots();
 
+    /**
+     * Get the line separator to use in the pom.xml.
+     *
+     * (https://issues.apache.org/jira/browse/MRELEASE-899).

Review comment:
       This is reduandant since it is in the commit message and blame.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-release] K-J-Henken commented on a change in pull request #62: [MRELEASE-899] Feature/lineseparator

Posted by GitBox <gi...@apache.org>.
K-J-Henken commented on a change in pull request #62:
URL: https://github.com/apache/maven-release/pull/62#discussion_r537334647



##########
File path: maven-release-plugin/src/main/java/org/apache/maven/plugins/release/PrepareReleaseMojo.java
##########
@@ -387,4 +403,27 @@ protected void prepareRelease( boolean generateReleasePoms )
         }
     }
 
+    private String resolveLineSeparator()
+    {
+        if ( lineSeparator  == null )
+        {
+            return System.getProperty( "line.separator" );
+        }
+
+        switch ( lineSeparator )

Review comment:
       Hey, i think the "same as source" option is a good idea in general but not as the default option because it would change the current behavior.
   Maybe I will implement it soon, but not in this request.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-release] K-J-Henken commented on a change in pull request #62: [MRELEASE-899] Feature/lineseparator

Posted by GitBox <gi...@apache.org>.
K-J-Henken commented on a change in pull request #62:
URL: https://github.com/apache/maven-release/pull/62#discussion_r819348535



##########
File path: maven-release-api/src/main/java/org/apache/maven/shared/release/config/ReleaseDescriptor.java
##########
@@ -567,6 +567,15 @@
      */
     String getAutoResolveSnapshots();
 
+    /**
+     * Get the line separator to use in the pom.xml.
+     *
+     * (https://issues.apache.org/jira/browse/MRELEASE-899).

Review comment:
       Fixed with https://github.com/apache/maven-release/pull/62/commits/349fe933a951cf1a879ac16d965c845784466397

##########
File path: maven-release-manager/src/main/mdo/release-descriptor.mdo
##########
@@ -604,6 +604,16 @@
           </description>
         </field>
 
+        <field>
+          <name>lineSeparator</name>
+          <version>3.0.0+</version>
+          <type>String</type>
+          <description>
+            Specifies the line separator to use in the pom.xml.
+            (https://issues.apache.org/jira/browse/MRELEASE-899)

Review comment:
       Fixed with https://github.com/apache/maven-release/pull/62/commits/349fe933a951cf1a879ac16d965c845784466397




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-release] K-J-Henken commented on a change in pull request #62: [MRELEASE-899] Feature/lineseparator

Posted by GitBox <gi...@apache.org>.
K-J-Henken commented on a change in pull request #62:
URL: https://github.com/apache/maven-release/pull/62#discussion_r553916385



##########
File path: maven-release-plugin/src/main/java/org/apache/maven/plugins/release/PrepareReleaseMojo.java
##########
@@ -387,4 +403,27 @@ protected void prepareRelease( boolean generateReleasePoms )
         }
     }
 
+    private String resolveLineSeparator()
+    {
+        if ( lineSeparator  == null )
+        {
+            return System.getProperty( "line.separator" );
+        }
+
+        switch ( lineSeparator )

Review comment:
       Would it be an alternative for you if I implement the "same as souce" option and set it as the default?

##########
File path: maven-release-plugin/src/main/java/org/apache/maven/plugins/release/PrepareReleaseMojo.java
##########
@@ -387,4 +403,27 @@ protected void prepareRelease( boolean generateReleasePoms )
         }
     }
 
+    private String resolveLineSeparator()
+    {
+        if ( lineSeparator  == null )
+        {
+            return System.getProperty( "line.separator" );
+        }
+
+        switch ( lineSeparator )

Review comment:
       Would it be an alternative for you if I **additionally** implement the "same as souce" option and set it as the default?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-release] K-J-Henken commented on a change in pull request #62: [MRELEASE-899] Feature/lineseparator

Posted by GitBox <gi...@apache.org>.
K-J-Henken commented on a change in pull request #62:
URL: https://github.com/apache/maven-release/pull/62#discussion_r553916385



##########
File path: maven-release-plugin/src/main/java/org/apache/maven/plugins/release/PrepareReleaseMojo.java
##########
@@ -387,4 +403,27 @@ protected void prepareRelease( boolean generateReleasePoms )
         }
     }
 
+    private String resolveLineSeparator()
+    {
+        if ( lineSeparator  == null )
+        {
+            return System.getProperty( "line.separator" );
+        }
+
+        switch ( lineSeparator )

Review comment:
       Would it be an alternative for you if I **additionally** implement the "same as souce" option and set it as the default?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-release] asfgit closed pull request #62: [MRELEASE-899] Feature/lineseparator

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #62:
URL: https://github.com/apache/maven-release/pull/62


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-release] K-J-Henken commented on a change in pull request #62: [MRELEASE-899] Feature/lineseparator

Posted by GitBox <gi...@apache.org>.
K-J-Henken commented on a change in pull request #62:
URL: https://github.com/apache/maven-release/pull/62#discussion_r553916385



##########
File path: maven-release-plugin/src/main/java/org/apache/maven/plugins/release/PrepareReleaseMojo.java
##########
@@ -387,4 +403,27 @@ protected void prepareRelease( boolean generateReleasePoms )
         }
     }
 
+    private String resolveLineSeparator()
+    {
+        if ( lineSeparator  == null )
+        {
+            return System.getProperty( "line.separator" );
+        }
+
+        switch ( lineSeparator )

Review comment:
       Would it be an alternative for you if I implement the "same as souce" option and set it as the default?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-release] michael-o commented on pull request #62: [MRELEASE-899] Feature/lineseparator

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #62:
URL: https://github.com/apache/maven-release/pull/62#issuecomment-1003706970


   Please rebase, I will reassess this for M6.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-release] michael-o commented on a change in pull request #62: [MRELEASE-899] Feature/lineseparator

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #62:
URL: https://github.com/apache/maven-release/pull/62#discussion_r537335786



##########
File path: maven-release-plugin/src/main/java/org/apache/maven/plugins/release/PrepareReleaseMojo.java
##########
@@ -387,4 +403,27 @@ protected void prepareRelease( boolean generateReleasePoms )
         }
     }
 
+    private String resolveLineSeparator()
+    {
+        if ( lineSeparator  == null )
+        {
+            return System.getProperty( "line.separator" );
+        }
+
+        switch ( lineSeparator )

Review comment:
       I prefer not to have an option at all: Same as source always, implicit.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-release] Captain-P-Goldfish commented on pull request #62: [MRELEASE-899] Feature/lineseparator

Posted by GitBox <gi...@apache.org>.
Captain-P-Goldfish commented on pull request #62:
URL: https://github.com/apache/maven-release/pull/62#issuecomment-978874898


   ping.
   Could anyone please merge this commit into the master-branch?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-release] jtnord commented on a change in pull request #62: [MRELEASE-899] Feature/lineseparator

Posted by GitBox <gi...@apache.org>.
jtnord commented on a change in pull request #62:
URL: https://github.com/apache/maven-release/pull/62#discussion_r535076817



##########
File path: maven-release-plugin/src/main/java/org/apache/maven/plugins/release/PrepareReleaseMojo.java
##########
@@ -387,4 +403,27 @@ protected void prepareRelease( boolean generateReleasePoms )
         }
     }
 
+    private String resolveLineSeparator()
+    {
+        if ( lineSeparator  == null )
+        {
+            return System.getProperty( "line.separator" );
+        }
+
+        switch ( lineSeparator )

Review comment:
       it would be really great if there was a "same as source" option that sniffed the line endings in the current projects `pom.xml` and used that as the default rather than the jvm default.  this would fix MRELEASE-899 without having to make any changes, and should not affect any user who was not affected by MRELEASE-899 (or its inverse that \n would be used on *nix when a \r\n file was checked out) already.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org