You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@geode.apache.org by "kirklund (GitHub)" <gi...@apache.org> on 2018/09/26 17:26:02 UTC

[GitHub] [geode] kirklund opened pull request #2517: GEODE-2644: Cleanup logging classes


[ Full content available at: https://github.com/apache/geode/pull/2517 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] kirklund commented on pull request #2517: GEODE-2644: Cleanup logging classes

Posted by "kirklund (GitHub)" <gi...@apache.org>.

@pdxrunner @sboorlagadda I'm actually not sure how to handle some of this logic:
```java
      if (endChar == '\n') {
        // trim off the \n since println will do it for us
        wordEnd--;
        if (wordEnd > 0 && target.charAt(wordEnd - 1) == '\r') {
          wordEnd--;
        }
```
I can't change that to System.getProperty("line.separator")

How about I merge this in and then I file a ticket to change all \n to be platform independent and I pair with one or both of you to fix everything related to \n?

[ Full content available at: https://github.com/apache/geode/pull/2517 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] kirklund commented on pull request #2517: GEODE-2644: Cleanup logging classes

Posted by "kirklund (GitHub)" <gi...@apache.org>.

I think we should have change every \n in the code base to be System.getProperty("line.separator") in one PR, but I'll go ahead and change this class.

[ Full content available at: https://github.com/apache/geode/pull/2517 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pdxrunner commented on pull request #2517: GEODE-2644: Cleanup logging classes

Posted by "pdxrunner (GitHub)" <gi...@apache.org>.

Sorry, my original comment was too terse. My intent was as @sboorlagadda said. All part of the effort to remove platform dependencies like pathname separators, end of line conventions, classpath separators, etc.

[ Full content available at: https://github.com/apache/geode/pull/2517 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] kirklund commented on pull request #2517: GEODE-2644: Cleanup logging classes

Posted by "kirklund (GitHub)" <gi...@apache.org>.

What do you want to replace the hardcoded newlines with? Without them the stack trace will be all on one line.

[ Full content available at: https://github.com/apache/geode/pull/2517 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pdxrunner commented on pull request #2517: GEODE-2644: Cleanup logging classes

Posted by "pdxrunner (GitHub)" <gi...@apache.org>.

Sounds OK.

[ Full content available at: https://github.com/apache/geode/pull/2517 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] kirklund closed pull request #2517: GEODE-2644: Cleanup logging classes

Posted by "kirklund (GitHub)" <gi...@apache.org>.

[ pull request closed by kirklund ]

[ Full content available at: https://github.com/apache/geode/pull/2517 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] sboorlagadda commented on pull request #2517: GEODE-2644: Cleanup logging classes

Posted by "sboorlagadda (GitHub)" <gi...@apache.org>.

you can use `System.getProperty("line.separator")` so that its platform agnostic.

[ Full content available at: https://github.com/apache/geode/pull/2517 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] kirklund commented on pull request #2517: GEODE-2644: Cleanup logging classes

Posted by "kirklund (GitHub)" <gi...@apache.org>.

I think we should change every \n in the code base to be System.getProperty("line.separator") in one PR, but I'll go ahead and change this class.

[ Full content available at: https://github.com/apache/geode/pull/2517 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] sboorlagadda commented on pull request #2517: GEODE-2644: Cleanup logging classes

Posted by "sboorlagadda (GitHub)" <gi...@apache.org>.

use `System.getProperty("line.separator")` so that its platform agnostic.

[ Full content available at: https://github.com/apache/geode/pull/2517 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pdxrunner commented on pull request #2517: GEODE-2644: Cleanup logging classes

Posted by "pdxrunner (GitHub)" <gi...@apache.org>.

I think we should get rid of all these hardcoded newlines.

[ Full content available at: https://github.com/apache/geode/pull/2517 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org