[ 
https://issues.apache.org/jira/browse/HIVE-20807?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16684486#comment-16684486
 ] 

Miklos Gergely commented on HIVE-20807:
---------------------------------------

Thank you [~sershe]!

About the line lengths: there is kind of a confusion here, the following are 
contradicting each other:

https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-CodingConventions

https://github.com/apache/hive/blob/bc39c49988c8a5d881a23ed7dd5d4adba0509ee9/checkstyle/checkstyle.xml#L163

The latter was introduced this January by [~kgyrtkirk], reviewed by 
[~ashutoshc], and there is some discussion about it at HIVE-18222.

I personally prefer 120, but will go with 100 and fix all lines going forward 
in may patches to that, but then we should modify the checkstyle.xml as well.

> Refactor LlapStatusServiceDriver
> --------------------------------
>
>                 Key: HIVE-20807
>                 URL: https://issues.apache.org/jira/browse/HIVE-20807
>             Project: Hive
>          Issue Type: Improvement
>          Components: Hive
>    Affects Versions: 4.0.0
>            Reporter: Miklos Gergely
>            Assignee: Miklos Gergely
>            Priority: Major
>             Fix For: 4.0.0
>
>         Attachments: HIVE-20807.01.patch, HIVE-20807.02.patch, 
> HIVE-20807.03.patch, HIVE-20807.04.patch, HIVE-20807.05.patch, 
> HIVE-20807.06.patch
>
>
> LlapStatusServiceDriver is the class used to determine if LLAP has started. 
> The following problems should be solved by refactoring:
> 1. The main class is more than 800 lines long,should be cut into multiple 
> smaller classes.
> 2. The current design makes it extremely hard to write unit tests.
> 3. There are some overcomplicated, over-engineered parts of the code.
> 4. Most of the code is under org.apache.hadoop.hive.llap.cli, but some parts 
> are under org.apache.hadoop.hive.llap.cli.status. The whole program could be 
> moved to the latter.
> 5. LlapStatusHelpers serves as a class for holding classes, which doesn't 
> make much sense.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to