[GitHub] incubator-carbondata pull request #471: [CARBONDATA-566] clean up code for c...

[qiuchenjian-2]

GitHub user jackylk opened a pull request:

    https://github.com/apache/incubator-carbondata/pull/471

    [CARBONDATA-566] clean up code for carbon-spark2 module

    Clean up code suggested by running IntelliJ Analyzer

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

    $ git pull https://github.com/jackylk/incubator-carbondata clean

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

    https://github.com/apache/incubator-carbondata/pull/471.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 #471
   
----
commit 9a434ccc3544122e1da56a6568f6fbfe281fe460
Author: jackylk <[hidden email]>
Date:   2016-12-27T03:27:59Z

    remove redundant declaration

commit fb9b514fb7178394c7bd2a4f4546e6ef64d57e61
Author: jackylk <[hidden email]>
Date:   2016-12-27T03:53:03Z

    clean up

commit 342681a02c89a4a87e58a0693ad6ba9e1d93a3ce
Author: jackylk <[hidden email]>
Date:   2016-12-27T04:02:34Z

    clean up

commit 1113d340d22e2f410366d3bf018e3f9fb44317cb
Author: jackylk <[hidden email]>
Date:   2016-12-27T04:16:02Z

    clean up RDDFactory

----


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/471
 
    Build Failed  with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/334/



---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/471
 
    Build Failed  with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/336/



---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/471
 
    Build Failed  with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/337/



---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/471
 
    Build Success with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/338/



---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user chenliang613 commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/471#discussion_r93921710
 
    --- Diff: integration/spark2/src/main/java/org/apache/carbondata/spark/vectorreader/ColumnarVectorWrapper.java ---
    @@ -21,7 +21,7 @@
     import org.apache.spark.sql.execution.vectorized.ColumnVector;
     import org.apache.spark.sql.types.Decimal;
     
    -public class ColumnarVectorWrapper implements CarbonColumnVector {
    +class ColumnarVectorWrapper implements CarbonColumnVector {
    --- End diff --
   
    why remove public ?  
    In the future, the class may be used by other class. suggest keep public.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/471#discussion_r93936025
 
    --- Diff: integration/spark2/src/main/java/org/apache/carbondata/spark/vectorreader/ColumnarVectorWrapper.java ---
    @@ -21,7 +21,7 @@
     import org.apache.spark.sql.execution.vectorized.ColumnVector;
     import org.apache.spark.sql.types.Decimal;
     
    -public class ColumnarVectorWrapper implements CarbonColumnVector {
    +class ColumnarVectorWrapper implements CarbonColumnVector {
    --- End diff --
   
    This is used internally only, not API. So it should not be public.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user chenliang613 commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/471#discussion_r93937123
 
    --- Diff: integration/spark2/src/main/java/org/apache/carbondata/spark/vectorreader/ColumnarVectorWrapper.java ---
    @@ -21,7 +21,7 @@
     import org.apache.spark.sql.execution.vectorized.ColumnVector;
     import org.apache.spark.sql.types.Decimal;
     
    -public class ColumnarVectorWrapper implements CarbonColumnVector {
    +class ColumnarVectorWrapper implements CarbonColumnVector {
    --- End diff --
   
    i mean that other package's class may use the class also.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/471#discussion_r93937751
 
    --- Diff: integration/spark2/src/main/java/org/apache/carbondata/spark/vectorreader/ColumnarVectorWrapper.java ---
    @@ -21,7 +21,7 @@
     import org.apache.spark.sql.execution.vectorized.ColumnVector;
     import org.apache.spark.sql.types.Decimal;
     
    -public class ColumnarVectorWrapper implements CarbonColumnVector {
    +class ColumnarVectorWrapper implements CarbonColumnVector {
    --- End diff --
   
    Because this is class is for vectorized reader feature in spark2 integration and It is only used in `VectorizedCarbonRecordReader` in the same package. It can be package-private instead of public


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/471
 
    LGTM


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-carbondata/pull/471


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---