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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

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

GitBox

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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

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

GitBox

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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

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

GitBox
In reply to this post by GitBox

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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

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

GitBox
In reply to this post by GitBox

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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

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

GitBox
In reply to this post by GitBox

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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

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

GitBox
In reply to this post by GitBox

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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

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

GitBox
In reply to this post by GitBox

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:
[hidden email]