You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by PascalSchumacher <gi...@git.apache.org> on 2016/11/06 18:20:33 UTC

[GitHub] commons-lang pull request #209: LANG-1164: allow ToStringStyle to omitNulls

GitHub user PascalSchumacher opened a pull request:

    https://github.com/apache/commons-lang/pull/209

    LANG-1164: allow ToStringStyle to omitNulls

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/PascalSchumacher/commons-lang toStringOmitNulls

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/commons-lang/pull/209.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #209
    
----
commit bd9505a01cef8cb6e49eff20abe221454be13bbd
Author: Pascal Schumacher <pa...@gmx.net>
Date:   2015-11-08T19:58:53Z

    LANG-1164: allow ToStringStyle to omitNulls

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #209: LANG-1164: allow ToStringStyle to omitNulls

Posted by PascalSchumacher <gi...@git.apache.org>.
Github user PascalSchumacher commented on the issue:

    https://github.com/apache/commons-lang/pull/209
  
    Thanks. :smile: 
    
    You won't need this to exclude `null` values when using `ReflectionToStringBuilder`, see https://github.com/apache/commons-lang/pull/259
    
    This will only be useful for custom `ToStringBuilder` implementations. I'm not sure that is is still worth adding given ReflectionToStringBuilder#excludeNullValues`. What do you think?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #209: LANG-1164: allow ToStringStyle to omitNulls

Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:

    https://github.com/apache/commons-lang/pull/209
  
    
    [![Coverage Status](https://coveralls.io/builds/8691246/badge)](https://coveralls.io/builds/8691246)
    
    Coverage increased (+0.02%) to 93.58% when pulling **bd9505a01cef8cb6e49eff20abe221454be13bbd on PascalSchumacher:toStringOmitNulls** into **05647d46e9ac2bf674b320e8467616aa72954f3e on apache:master**.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #209: LANG-1164: allow ToStringStyle to omitNulls

Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on the issue:

    https://github.com/apache/commons-lang/pull/209
  
    Oh, TIL. I think we can't close this pull request, and wait in case a user needs this feature :+1: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang pull request #209: LANG-1164: allow ToStringStyle to omitNulls

Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on a diff in the pull request:

    https://github.com/apache/commons-lang/pull/209#discussion_r114114198
  
    --- Diff: src/main/java/org/apache/commons/lang3/builder/ToStringStyle.java ---
    @@ -467,6 +472,7 @@ protected void removeLastFieldSeparator(final StringBuffer buffer) {
          *  for summary info, <code>null</code> for style decides
          */
         public void append(final StringBuffer buffer, final String fieldName, final Object value, final Boolean fullDetail) {
    +        if (omitNulls && value == null) return;
    --- End diff --
    
    With our current checkstyle checks, this will create an error (error in Checkstyle, not a build error).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #209: LANG-1164: allow ToStringStyle to omitNulls

Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on the issue:

    https://github.com/apache/commons-lang/pull/209
  
    +1 lgtm
    
    Looks useful, I've wanted that feature a couple of times when using ReflectionToStringBuilder in the past.
    
    Merging the code locally, the build passes, all reports look good but Chekstyle.
    
    ```
    org/apache/commons/lang3/builder/ToStringStyle.java
    Severity 	Category 	Rule 	Message 	Line
     Error 	blocks 	NeedBraces 	'if' construct must use '{}'s. 	474
     Error 	blocks 	NeedBraces 	'if' construct must use '{}'s. 	908
     Error 	blocks 	NeedBraces 	'if' construct must use '{}'s. 	1005
     Error 	blocks 	NeedBraces 	'if' construct must use '{}'s. 	1067
     Error 	blocks 	NeedBraces 	'if' construct must use '{}'s. 	1129
     Error 	blocks 	NeedBraces 	'if' construct must use '{}'s. 	1191
     Error 	blocks 	NeedBraces 	'if' construct must use '{}'s. 	1253
     Error 	blocks 	NeedBraces 	'if' construct must use '{}'s. 	1315
     Error 	blocks 	NeedBraces 	'if' construct must use '{}'s. 	1377
     Error 	blocks 	NeedBraces 	'if' construct must use '{}'s. 	1439
    ```
    
    The inlined if fails with our current checkstyle rules. Other than that, all looks good :-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang pull request #209: LANG-1164: allow ToStringStyle to omitNulls

Posted by PascalSchumacher <gi...@git.apache.org>.
Github user PascalSchumacher closed the pull request at:

    https://github.com/apache/commons-lang/pull/209


---

[GitHub] commons-lang issue #209: LANG-1164: allow ToStringStyle to omitNulls

Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:

    https://github.com/apache/commons-lang/pull/209
  
    
    [![Coverage Status](https://coveralls.io/builds/8692938/badge)](https://coveralls.io/builds/8692938)
    
    Coverage increased (+0.02%) to 93.58% when pulling **7bddc8759efdde74f6c3da334b891bee840508f0 on PascalSchumacher:toStringOmitNulls** into **05647d46e9ac2bf674b320e8467616aa72954f3e on apache:master**.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---