You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by HarshSharma8 <gi...@git.apache.org> on 2017/02/24 12:10:36 UTC

[GitHub] spark pull request #17058: Refactored code to remove null representation

GitHub user HarshSharma8 opened a pull request:

    https://github.com/apache/spark/pull/17058

    Refactored code to remove null representation

    ## What changes were proposed in this pull request?
    
    Removed the null value that was returned directly with null representation and returned in a better way.
    
    ## How was this patch tested?
    
    The result remains same, its just a refactoring of code.
    
    Please review http://spark.apache.org/contributing.html before opening a pull request.


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

    $ git pull https://github.com/HarshSharma8/spark refactor/removeNull

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

    https://github.com/apache/spark/pull/17058.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 #17058
    
----
commit 11cbd9d5f58e19f04743daffcbcd985327079817
Author: Harsh Sharma <ha...@knoldus.com>
Date:   2017-02-24T12:08:12Z

    Refactored code to remove null representation

----


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17058: Refactored code to remove null representation

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

    https://github.com/apache/spark/pull/17058
  
    It seems too tirival. The `default` seems even always `null`. I don't think this is an improvement if there is no other specific reason. The original codes are more readable to me.


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17058: Refactored code to remove null representation

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

    https://github.com/apache/spark/pull/17058
  
    Can one of the admins verify this patch?


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17058: Refactored code to remove null representation

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

    https://github.com/apache/spark/pull/17058


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17058: Refactored code to remove null representation

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

    https://github.com/apache/spark/pull/17058
  
    `var` is mut�ble and the "default value" seems not exposed to users and being changed.


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17058: Refactored code to remove null representation

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

    https://github.com/apache/spark/pull/17058
  
    May be this is right and readable from your point of view, this code is
    returning nothing else, just null again and again as default value if user
    along its other values are not present.
    From my point of view i just let the user know about the default value that
    i am going to use.
    
    On Feb 24, 2017 6:04 PM, "Hyukjin Kwon" <no...@github.com> wrote:
    
    > It seems too tirival. The default seems even always null. I don't think
    > this is an improvement if there is no other specific reason. The original
    > codes are more readable to me.
    >
    > \u2014
    > You are receiving this because you authored the thread.
    > Reply to this email directly, view it on GitHub
    > <https://github.com/apache/spark/pull/17058#issuecomment-282280760>, or mute
    > the thread
    > <https://github.com/notifications/unsubscribe-auth/AKIiQL-Ob-HDj0Jz8ykRo_7frY7qFhT1ks5rfs42gaJpZM4MLI9I>
    > .
    >



---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17058: Refactored code to remove null representation

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

    https://github.com/apache/spark/pull/17058
  
    @HarshSharma8  this is not an improvement, please close this. This is not the kind of PR that's worth submitting. See http://spark.apache.org/contributing.html


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org