You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by samdvr <gi...@git.apache.org> on 2018/09/30 15:44:33 UTC

[GitHub] spark pull request #22596: Fix lint failure in 2.2

GitHub user samdvr opened a pull request:

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

    Fix lint failure in 2.2 

    ## What changes were proposed in this pull request?
    
    Line length fixes and 
    
    ## How was this patch tested?
    
    Manually verified, but will ensure jenkins lint passes before merging 


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

    $ git pull https://github.com/samdvr/spark SPARK-25576

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

    https://github.com/apache/spark/pull/22596.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 #22596
    
----
commit c4b6920ed24fd6398da27b978380427e3b0cb62a
Author: Sam Davarnia <>
Date:   2018-09-30T14:36:33Z

    fix liniting errors

----


---

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


[GitHub] spark pull request #22596: Fix lint failure in 2.2

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

    https://github.com/apache/spark/pull/22596#discussion_r221688986
  
    --- Diff: core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeInMemorySorter.java ---
    @@ -174,9 +174,12 @@ public void reset() {
         if (consumer != null) {
           consumer.freeArray(array);
           // the call to consumer.allocateArray may trigger a spill
    -      // which in turn access this instance and eventually re-enter this method and try to free the array again.
    -      // by setting the array to null and its length to 0 we effectively make the spill code-path a no-op.
    -      // setting the array to null also indicates that it has already been de-allocated which prevents a double de-allocation in free().
    +      // which in turn access this instance and eventually re-enter this method 
    +      // and try to free the array again.
    +      // by setting the array to null and its length to 0 
    --- End diff --
    
    `by` -> `By`?


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22596: Fix lint failure in 2.2

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

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


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22596: Fix lint failure in 2.2

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

    https://github.com/apache/spark/pull/22596
  
    **[Test build #96808 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96808/testReport)** for PR 22596 at commit [`c4b6920`](https://github.com/apache/spark/commit/c4b6920ed24fd6398da27b978380427e3b0cb62a).


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    **[Test build #96823 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96823/testReport)** for PR 22596 at commit [`009d4cf`](https://github.com/apache/spark/commit/009d4cf0529917de32483f9d7686b7dcdbe6e400).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark pull request #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596#discussion_r221835152
  
    --- Diff: common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java ---
    @@ -63,7 +63,6 @@ public void basicTest() {
         checkBasic("hello", 5); // 5 * 1 byte chars
         checkBasic("大 千 世 界", 7);
         checkBasic("︽﹋%", 3); // 3 * 3 bytes chars
    -    checkBasic("\uD83E\uDD19", 1); // 4 bytes char
    --- End diff --
    
    We should not remove test coverage.


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96858/
    Test PASSed.


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    **[Test build #96858 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96858/testReport)** for PR 22596 at commit [`07156d4`](https://github.com/apache/spark/commit/07156d4d8d3ac40b47a825ecb4fa01bf7eaae1ad).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    **[Test build #96835 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96835/testReport)** for PR 22596 at commit [`2e8836b`](https://github.com/apache/spark/commit/2e8836ba0cab70243a8b5ef8845d045d65856aa1).


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96835/
    Test PASSed.


---

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


[GitHub] spark issue #22596: Fix lint failure in 2.2

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

    https://github.com/apache/spark/pull/22596
  
    Can you link the JIRA https://issues.apache.org/jira/browse/SPARK-25576 ? Please see https://spark.apache.org/contributing.html


---

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


[GitHub] spark issue #22596: Fix lint failure in 2.2

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

    https://github.com/apache/spark/pull/22596
  
    **[Test build #96808 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96808/testReport)** for PR 22596 at commit [`c4b6920`](https://github.com/apache/spark/commit/c4b6920ed24fd6398da27b978380427e3b0cb62a).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22596: Fix lint failure in 2.2

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

    https://github.com/apache/spark/pull/22596
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    **[Test build #96835 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96835/testReport)** for PR 22596 at commit [`2e8836b`](https://github.com/apache/spark/commit/2e8836ba0cab70243a8b5ef8845d045d65856aa1).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #22596: Fix lint failure in 2.2

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

    https://github.com/apache/spark/pull/22596
  
    ok to test


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96823/
    Test PASSed.


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    @dongjoon-hyun merged, thank you, did not know how to skip lint rule there.


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    BTW, I chose the most official one, `Sam Davarnia <sd...@apple.com>`. But, it seems that your GitHub is not happy with that. Next time, you had better be consistent in your commit message.
    ```
    Lead-authored-by: Sam Davarnia <sd...@apple.com>
    Co-authored-by: Sam Davarnia <>
    Co-authored-by: Dongjoon Hyun <do...@apache.org>
    Co-authored-by: Sam Davarnia <sa...@users.noreply.github.com>
    Signed-off-by: Dongjoon Hyun <do...@apache.org>
    ```


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    Since it's merged into `branch-2.2`, could you close this PR? GitHub only closes the PRs against `master` branch. :)


---

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


[GitHub] spark issue #22596: Fix lint failure in 2.2

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

    https://github.com/apache/spark/pull/22596
  
    Thank you for your first contribution, @samdvr . Could you update the title, `[SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure`?


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    Thanks will do!


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    **[Test build #96823 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96823/testReport)** for PR 22596 at commit [`009d4cf`](https://github.com/apache/spark/commit/009d4cf0529917de32483f9d7686b7dcdbe6e400).


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    @dongjoon-hyun updated per your comments, thank you.


---

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


[GitHub] spark issue #22596: Fix lint failure in 2.2

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

    https://github.com/apache/spark/pull/22596
  
    @HyukjinKwon linked the JIRA to this PR and tests have passed.


---

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


[GitHub] spark issue #22596: Fix lint failure in 2.2

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

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


---

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


[GitHub] spark pull request #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

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


---

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


[GitHub] spark issue #22596: [SPARK-25576][BUILD][BRANCH-2.2] Fix lint failure

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

    https://github.com/apache/spark/pull/22596
  
    **[Test build #96858 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96858/testReport)** for PR 22596 at commit [`07156d4`](https://github.com/apache/spark/commit/07156d4d8d3ac40b47a825ecb4fa01bf7eaae1ad).


---

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


[GitHub] spark issue #22596: Fix lint failure in 2.2

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

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


---

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


[GitHub] spark pull request #22596: Fix lint failure in 2.2

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

    https://github.com/apache/spark/pull/22596#discussion_r221689165
  
    --- Diff: core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeInMemorySorter.java ---
    @@ -174,9 +174,12 @@ public void reset() {
         if (consumer != null) {
           consumer.freeArray(array);
           // the call to consumer.allocateArray may trigger a spill
    -      // which in turn access this instance and eventually re-enter this method and try to free the array again.
    -      // by setting the array to null and its length to 0 we effectively make the spill code-path a no-op.
    -      // setting the array to null also indicates that it has already been de-allocated which prevents a double de-allocation in free().
    +      // which in turn access this instance and eventually re-enter this method 
    +      // and try to free the array again.
    +      // by setting the array to null and its length to 0 
    +      // we effectively make the spill code-path a no-op.
    +      // setting the array to null also indicates that it has already been 
    --- End diff --
    
    `setting` -> `Setting`?


---

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


[GitHub] spark issue #22596: Fix lint failure in 2.2

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

    https://github.com/apache/spark/pull/22596
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96808/
    Test PASSed.


---

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