[GitHub] carbondata pull request #3036: [CARBONDATA-3208]Remove unused parameters and...

[qiuchenjian-2]

GitHub user runzhliu opened a pull request:

    https://github.com/apache/carbondata/pull/3036

    [CARBONDATA-3208]Remove unused parameters and imports from code

    Be sure to do all of the following checklist to help us incorporate
    your contribution quickly and easily:
   
     - [x] Any interfaces changed?
     
     - [x] Any backward compatibility impacted?
     
     - [x] Document update required?
   
     - [x] Testing done
            Please provide details on
            - Whether new unit test cases have been added or why no new tests are required?
            - How it is tested? Please attach test report.
            - Is it a performance related change? Please attach the performance test report.
            - Any additional information to help reviewers in testing this change.
           
     - [x] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
   


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

    $ git pull https://github.com/runzhliu/carbondata runzhliu

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

    https://github.com/apache/carbondata/pull/3036.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 #3036
   
----
commit 4213886534fdf68a143060776afa47d269fac782
Author: Oscar <runzhliu@...>
Date:   2018-12-27T23:24:06Z

    [CARBONDATA-3208]Remove unused parameters and imports from code

----


---

[qiuchenjian-2]

Github user runzhliu commented on the issue:

    https://github.com/apache/carbondata/pull/3036
 
    Hi @xubo245 , can you please take a look?


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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


---

[qiuchenjian-2]

Github user qiuchenjian commented on the issue:

    https://github.com/apache/carbondata/pull/3036
 
    @xuchuanyin   @xubo245 @QiangCai  @jackylk  Can we enable checkstyle rule of avoiding unused imports，i think it's necessary to keep good code style and reduce the dependency of a class on the number of jar packages,  the rule may be is   <module name="UnusedImports"/>


---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/3036
 
    @qiuchenjian I tried it before, but there are some limit, not easy to support it. If you have better way to support it, you can raise a PR to implement it.


---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/3036
 
    add to whitelist


---

[qiuchenjian-2]

Github user qiuchenjian commented on the issue:

    https://github.com/apache/carbondata/pull/3036
 
    @xubo245 OK, i'll try


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3036#discussion_r244461155
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/spark/PartitionFactory.scala ---
    @@ -32,7 +32,7 @@ object PartitionFactory {
           case PartitionType.LIST => new ListPartitioner(partitionInfo)
           case PartitionType.RANGE => new RangePartitioner(partitionInfo)
           case partitionType =>
    -        throw new CarbonDataLoadingException(s"Unsupport partition type: $partitionType")
    +        throw new CarbonDataLoadingException(s"Unsupported partition type: $partitionType")
    --- End diff --
   
    Can you check it in whole project, there are another two.


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3036
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2075/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3036#discussion_r244461274
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/spark/rdd/CarbonGlobalDictionaryRDD.scala ---
    @@ -483,12 +483,12 @@ class CarbonGlobalDictionaryGenerateRDD(
     }
     
     /**
    - * Set column dictionry patition format
    --- End diff --
   
    Have you check it in the whole project? patition has 25+


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3036#discussion_r244461434
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/spark/util/GlobalDictionaryUtil.scala ---
    @@ -387,7 +382,7 @@ object GlobalDictionaryUtil {
        * @param table         carbon table identifier
        * @param colName       user specified  column name for predefined dict
        * @param colDictPath   column dictionary file path
    -   * @param parentDimName parent dimenion for complex type
    +   * @param parentDimName parent dimension for complex type
    --- End diff --
   
    Have you check it in the whole project?


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3036
 
    Build Failed  with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10329/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3036#discussion_r244462534
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/spark/PartitionFactory.scala ---
    @@ -32,7 +32,7 @@ object PartitionFactory {
           case PartitionType.LIST => new ListPartitioner(partitionInfo)
           case PartitionType.RANGE => new RangePartitioner(partitionInfo)
           case partitionType =>
    -        throw new CarbonDataLoadingException(s"Unsupport partition type: $partitionType")
    +        throw new CarbonDataLoadingException(s"Unsupported partition type: $partitionType")
    --- End diff --
   
    Sure, I got another three.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3036#discussion_r244462598
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/spark/rdd/CarbonGlobalDictionaryRDD.scala ---
    @@ -483,12 +483,12 @@ class CarbonGlobalDictionaryGenerateRDD(
     }
     
     /**
    - * Set column dictionry patition format
    --- End diff --
   
    OK


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3036#discussion_r244463587
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/spark/util/GlobalDictionaryUtil.scala ---
    @@ -387,7 +382,7 @@ object GlobalDictionaryUtil {
        * @param table         carbon table identifier
        * @param colName       user specified  column name for predefined dict
        * @param colDictPath   column dictionary file path
    -   * @param parentDimName parent dimenion for complex type
    +   * @param parentDimName parent dimension for complex type
    --- End diff --
   
    OK


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3036
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2280/



---

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/3036
 
    @runzhliu  please correct the PR title.


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3036
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2076/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3036
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2281/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3036
 
    Build Failed  with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10330/



---