You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by "Carl Steinbach (JIRA)" <ji...@apache.org> on 2010/02/02 05:54:19 UTC
[jira] Created: (HIVE-1123) Checkstyle fixes
Checkstyle fixes
----------------
Key: HIVE-1123
URL: https://issues.apache.org/jira/browse/HIVE-1123
Project: Hadoop Hive
Issue Type: Task
Reporter: Carl Steinbach
Assignee: Carl Steinbach
Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Ashish Thusoo (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12829816#action_12829816 ]
Ashish Thusoo commented on HIVE-1123:
-------------------------------------
Apart from the indentation of throws clause is there any other major sticking point. Personally speaking I don't have a strong preference for the indentation of throws. Going with 2 indents probably makes it easier for eclipse to catch this. @Carl I do think that there is value in publishing the entire set of rules that you have used.
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HIVE-1123) Checkstyle fixes
Posted by "Zheng Shao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Zheng Shao updated HIVE-1123:
-----------------------------
Status: Open (was: Patch Available)
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.patch, HIVE-1123.common.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.patch, HIVE-1123.serde.patch, HIVE-1123.service.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Carl Steinbach updated HIVE-1123:
---------------------------------
Attachment: HIVE-1123.service.patch
HIVE-1123.serde.patch
HIVE-1123.ql.patch
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.patch, HIVE-1123.common.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.patch, HIVE-1123.serde.patch, HIVE-1123.service.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Carl Steinbach updated HIVE-1123:
---------------------------------
Attachment: HIVE-1123.contrib.2.patch
HIVE-1123.common.2.patch
HIVE-1123.cli.2.patch
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.patch, HIVE-1123.serde.patch, HIVE-1123.service.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Zheng Shao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12829409#action_12829409 ]
Zheng Shao commented on HIVE-1123:
----------------------------------
The sticking point is that the patch is too big, and there is not a short list of rules for your changes.
I agree conforming to checkstyle in general is a good thing to do but there might be small rules here and there that we might not want to follow. Examples include the indentation rule (which you also mentioned personally you like the 4-space approach). There might be more but it's hard for me to catch them all.
If you can give me a full list of the rules of the changes you made, it will also speed up the review/commits.
I would ask other guys on the team to take a look at these patches. If we can get a consensus we will commit them soon.
In the future, it would also be great if you can make these patches one by one to save the effort of (potentially) redoing some of them because of conflicts.
Once we agree on the rules of the changes, it should be very fast to review/commit the other patches.
Does that sound good?
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Zheng Shao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12828969#action_12828969 ]
Zheng Shao commented on HIVE-1123:
----------------------------------
Another question. Is it recommended NOT to have the curly braces for a case statement?
{code}
switch (type) {
- case BYTE: {
+ case BYTE:
tbIn.readByte((ByteWritable) w);
break;
- }
- case BOOL: {
+ case BOOL:
tbIn.readBoolean((BooleanWritable) w);
break;
- }
{code}
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.patch, HIVE-1123.common.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.patch, HIVE-1123.serde.patch, HIVE-1123.service.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (HIVE-1123) Checkstyle fixes
Posted by "Zheng Shao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Zheng Shao resolved HIVE-1123.
------------------------------
Resolution: Fixed
Fix Version/s: 0.6.0
Release Note: HIVE-1123. Checkstyle fixes. (Carl Steinbach via zshao)
Hadoop Flags: [Reviewed]
Committed. Thanks Carl!
Please do the follow-ups including wiki page for code guidelines, eclipse setup as soon as possible so that our developers can follow the new standard that you helped set up.
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Fix For: 0.6.0
>
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Carl Steinbach updated HIVE-1123:
---------------------------------
Attachment: HIVE-1123.checkstyle.patch
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Zheng Shao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12828931#action_12828931 ]
Zheng Shao commented on HIVE-1123:
----------------------------------
It seems the maximum line length is 100 or something?
Is Hadoop common using 80? Why don't we make it 80?
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.patch, HIVE-1123.common.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.patch, HIVE-1123.serde.patch, HIVE-1123.service.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Zheng Shao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12828937#action_12828937 ]
Zheng Shao commented on HIVE-1123:
----------------------------------
Please ignore my question... I asked the same question on HIVE-990.
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.patch, HIVE-1123.common.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.patch, HIVE-1123.serde.patch, HIVE-1123.service.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Carl Steinbach updated HIVE-1123:
---------------------------------
Attachment: HIVE-1123.metastore.2.patch
HIVE-1123.jdbc.2.patch
HIVE-1123.hwi.2.patch
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Carl Steinbach updated HIVE-1123:
---------------------------------
Component/s: Build Infrastructure
Release Note: (was: HIVE-1123. Checkstyle fixes. (Carl Steinbach via zshao))
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hive
> Issue Type: Task
> Components: Build Infrastructure
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Fix For: 0.6.0
>
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Carl Steinbach updated HIVE-1123:
---------------------------------
Attachment: HIVE-1123.service.2.patch
HIVE-1123.serde.2.patch
HIVE-1123.ql.2.patch
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12830019#action_12830019 ]
Carl Steinbach commented on HIVE-1123:
--------------------------------------
bq. I talked with a bunch of team members. We will wait till Friday noon for feedbacks and decide what to do with these patches then.
Sounds good!
bq. By the way, Carl, is it easy to configure eclipse NOT to produce checkstyle-alarming code? As far as I know, eclipse leaves spaces in blank lines.
You can configure Eclipse to automatically remove trailing whitespace on save by going to
Properties -> Java Editor -> Save Actions. I think it should be possible to to add this to
the project settings so that folks will have it automatically set when they import the
project into Eclipse. I will look into getting this setup, along with other formatting settings
that are compatible with the Checkstyle configuration.
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12829371#action_12829371 ]
Carl Steinbach commented on HIVE-1123:
--------------------------------------
I reverted the indentation changes and modified the Checkstyle configuration in order
to disable indentation checks. I did not revert the switch statement changes since I don't
have an automatic way of doing this nor a canonical list of where I made those changes.
I also maintain that switch statement braces should be avoided unless you really need
to define a new scope, in which case you probably shouldn't be using a switch statement
in the first place.
Zheng, please take a look. Thanks.
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12829829#action_12829829 ]
Carl Steinbach commented on HIVE-1123:
--------------------------------------
The following checks are common to the Hive checkstyle configuration and the default Sun checks configuration:
* [ArrayTypeStyle|http://checkstyle.sourceforge.net/config_misc.html#ArrayTypeStyle]
* [AvoidNestedBlocks|http://checkstyle.sourceforge.net/config_blocks.html#AvoidNestedBlocks]
* [AvoidStarImport|http://checkstyle.sourceforge.net/config_imports.html#AvoidStarImport]
* [ConstantName|http://checkstyle.sourceforge.net/config_naming.html#ConstantName]
* [EmptyBlock|http://checkstyle.sourceforge.net/config_blocks.html#EmptyBlock]
* [EmptyForIteratorPad|http://checkstyle.sourceforge.net/config_whitespace.html#EmptyForIteratorPad]
* [EmptyStatement|http://checkstyle.sourceforge.net/config_coding.html#EmptyStatement]
* [EqualsHashCode|http://checkstyle.sourceforge.net/config_coding.html#EqualsHashCode]
* [FileLength|http://checkstyle.sourceforge.net/config_sizes.html#FileLength]
* [FileTabCharacter|http://checkstyle.sourceforge.net/config_whitespace.html#FileTabCharacter]
* [FinalClass|http://checkstyle.sourceforge.net/config_design.html#FinalClass]
* [HiddenField|http://checkstyle.sourceforge.net/config_coding.html#HiddenField]
* [HideUtilityClassConstructor|http://checkstyle.sourceforge.net/config_design.html#HideUtilityClassConstructor]
* [InnerAssignment|http://checkstyle.sourceforge.net/config_coding.html#InnerAssignment]
* [JavadocPackage|http://checkstyle.sourceforge.net/config_javadoc.html#JavadocPackage]
* [JavadocStyle|http://checkstyle.sourceforge.net/config_javadoc.html#JavadocStyle]
* [JavadocType|http://checkstyle.sourceforge.net/config_javadoc.html#JavadocType]
** Only required for public types. Allow missing parameter tags.
* [LeftCurly|http://checkstyle.sourceforge.net/config_blocks.html#LeftCurly]
* [LineLength|http://checkstyle.sourceforge.net/config_sizes.html#LineLength]
** Increased to 100 characters.
* [LocalFinalVariableName|http://checkstyle.sourceforge.net/config_naming.html#LocalFinalVariableName]
* [LocalVariableName|http://checkstyle.sourceforge.net/config_naming.html#LocalVariableName]
* [MemberName|http://checkstyle.sourceforge.net/config_naming.html#MemberName]
* [MethodLength|http://checkstyle.sourceforge.net/config_sizes.html#MethodLength]
* [MethodName|http://checkstyle.sourceforge.net/config_naming.html#MethodName]
* [MethodParamPad|http://checkstyle.sourceforge.net/config_whitespace.html#MethodParamPad]
* [MissingSwitchDefault|http://checkstyle.sourceforge.net/config_coding.html#MissingSwitchDefault]
* [ModifierOrder|http://checkstyle.sourceforge.net/config_modifier.html#ModifierOrder]
* [NeedBraces|http://checkstyle.sourceforge.net/config_blocks.html#NeedBraces]
* [NewlineAtEndOfFile|http://checkstyle.sourceforge.net/config_misc.html#NewlineAtEndOfFile]
* [NoWhitespaceAfter|http://checkstyle.sourceforge.net/config_whitespace.html#NoWhitespaceAfter]
* [NoWhitespaceBefore|http://checkstyle.sourceforge.net/config_whitespace.html#NoWhitespaceBefore]
* [PackageName|http://checkstyle.sourceforge.net/config_naming.html#PackageName]
* [ParameterName|http://checkstyle.sourceforge.net/config_naming.html#ParameterName]
* [ParameterNumber|http://checkstyle.sourceforge.net/config_sizes.html#ParameterNumber]
* [ParenPad|http://checkstyle.sourceforge.net/config_whitespace.html#ParenPad]
* [RedundantModifer|http://checkstyle.sourceforge.net/config_modifier.html#RedundantModifier]
* [RedundantThrows|http://checkstyle.sourceforge.net/config_coding.html#RedundantThrows]
* [RightCurly|http://checkstyle.sourceforge.net/config_blocks.html#RightCurly]
* [SimplifyBooleanExpression|http://checkstyle.sourceforge.net/config_coding.html#SimplifyBooleanExpression]
* [SimplifyBooleanReturn|http://checkstyle.sourceforge.net/config_coding.html#SimplifyBooleanReturn]
* [StaticVariableName|http://checkstyle.sourceforge.net/config_naming.html#StaticVariableName]
* [TodoComment|http://checkstyle.sourceforge.net/config_misc.html#TodoComment]
* [TypeName|http://checkstyle.sourceforge.net/config_naming.html#TypeName]
* [TypecastParenPad|http://checkstyle.sourceforge.net/config_whitespace.html#TypecastParenPad]
* [UnusedImports|http://checkstyle.sourceforge.net/config_imports.html#UnusedImports]
* [VisbilityModifier|http://checkstyle.sourceforge.net/config_design.html#VisibilityModifier]
* [WhitespaceAfter|http://checkstyle.sourceforge.net/config_whitespace.html#WhitespaceAfter]
The following checks are present in the default Sun configuration but were removed from the configuration that Hadoop uses (which I used as a starting point for the Hive configuration):
* [AvoidInlineConditionals|http://checkstyle.sourceforge.net/config_coding.html#AvoidInlineConditionals]
* [DesignForExtension|http://checkstyle.sourceforge.net/config_design.html#DesignForExtension]
* [FinalParameters|http://checkstyle.sourceforge.net/config_misc.html#FinalParameters]
* [GenericWhitespace|http://checkstyle.sourceforge.net/config_whitespace.html#GenericWhitespace]
* [JavadocMethod|http://checkstyle.sourceforge.net/config_javadoc.html#JavadocMethod]
* [JavadocVariable|http://checkstyle.sourceforge.net/config_javadoc.html#JavadocVariable]
* [MagicNumber|http://checkstyle.sourceforge.net/config_coding.html#MagicNumber]
* [OperatorWrap|http://checkstyle.sourceforge.net/config_whitespace.html#OperatorWrap]
* [WhitespaceAround|http://checkstyle.sourceforge.net/config_whitespace.html#WhitespaceAround]
I added the following checks to the Hive configuration:
* [EqualsAvoidNull|http://checkstyle.sourceforge.net/config_coding.html#EqualsAvoidNull]
* [Header|http://checkstyle.sourceforge.net/config_header.html#Header]
** Checks for ASF license header.
* [MissingOverride|http://checkstyle.sourceforge.net/config_annotation.html#MissingOverride]
* [StringLiteralEquality|http://checkstyle.sourceforge.net/config_coding.html#StringLiteralEquality]
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Carl Steinbach updated HIVE-1123:
---------------------------------
Attachment: HIVE-1123.shims.2.patch
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Zheng Shao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12828976#action_12828976 ]
Zheng Shao commented on HIVE-1123:
----------------------------------
Hi Carl, if you want us to commit the changes asap, can you exclude these 2 changes for now? I mean the indentation problem and the case curly braces problem.
All other changes (spaces between tokens, javadoc, keywords, etc) seem good to me.
It will also be great if you can add the list of style changes you made into a file "checkstyle.txt" or something so that we can publicize the common code convention problems to all the developers and committers.
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.patch, HIVE-1123.common.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.patch, HIVE-1123.serde.patch, HIVE-1123.service.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "John Sichi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12830269#action_12830269 ]
John Sichi commented on HIVE-1123:
----------------------------------
+1 on what Ning said. I too prefer the second format for nested method calls.
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Zheng Shao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12828939#action_12828939 ]
Zheng Shao commented on HIVE-1123:
----------------------------------
It seems you made 2 spaces instead of 4 for the "throws Exception" line in the following example. Is that recommended by the standard?
{code}
private void myMethod(...)
throws Expection {
doSomeThing();
}
{code}
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.patch, HIVE-1123.common.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.patch, HIVE-1123.serde.patch, HIVE-1123.service.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12829404#action_12829404 ]
Carl Steinbach commented on HIVE-1123:
--------------------------------------
Things that I did automatically using the Eclipse formatter:
{noformat}
Line length:
Limit is 100
Spacing:
Spaces between operator and operands except i++
Spaces between if and (, for and (, while and (
No spaces between ( and the first operand, or the last operand and )
Same for {}
Curly braces:
Required even for single-line if
} else {
Code:
"Type[] var" instead of "Type var[]"
Imports:
Ordered alphabetically
{noformat}
Things I did mainly by hand:
{noformat}
Javadoc Comments:
Required for all public members (classes, methods) but not getter/setter.
The first line of each javadoc is the short description. Should end with "."
Each line of javadoc should start with capital letters.
Keyword order:
public static final
Declare the class as final if not intended to be inherited.
Code:
Default private constructor for Utility classes.
Capitalization:
Constant should be all capitalized
Classes should be first-letter capitalized
Variable names should be first-letter lowercased, with the first letter of each additional word capitalized.
Don't use _ in variable names.
{noformat}
bq. If it's automatic, shall we do it one rule by one rule instead of one package by one package?
A good idea, but many of the changes I made were not automatic, and starting over would require
me to throw away this work. I don't think I have the patience to redo it, at least not right now.
What are the sticking points preventing you from committing the current version of the patch? Anything
besides the switch brace issue?
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Carl Steinbach updated HIVE-1123:
---------------------------------
Attachment: HIVE-1123.metastore.patch
HIVE-1123.jdbc.patch
HIVE-1123.hwi.patch
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.patch, HIVE-1123.common.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.patch, HIVE-1123.serde.patch, HIVE-1123.service.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Zheng Shao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12829374#action_12829374 ]
Zheng Shao commented on HIVE-1123:
----------------------------------
Hi Carl, is it true that without the new scopes in the switch clause, variables name in the different "case" cannot overlap?
For example, the following
{code}
switch(n) {
case 0:
int i = 3;
case 1:
int i = 2;
System.out.println(i);
}
{code}
What do you think we should do in this case?
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Zheng Shao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12829390#action_12829390 ]
Zheng Shao commented on HIVE-1123:
----------------------------------
For these changes, shall we do it by the *type* of the change instead of the packages?
The reason is that some of the changes we might want to discuss more with more people involved, but the others are good to go.
Here is the short list of what I see from the patch. I think most people will agree to these items.
{code}
Line length:
Limit is 100
Spacing:
Spaces between operator and operands except i++
Spaces between if and (, for and (, while and (
No spaces between ( and the first operand, or the last operand and )
Same for {}
Curly braces:
Required even for single-line if
} else {
Javadoc Comments:
Required for all public members (classes, methods) but not getter/setter.
The first line of each javadoc is the short description. Should end with "."
Each line of javadoc should start with capital letters.
Keyword order:
public static final
Declare the class as final if not intended to be inherited.
Code:
Default private constructor for Utility classes.
"Type[] var" instead of "Type var[]"
Capitalization:
Constant should be all capitalized
Classes should be first-letter capitalized
Variable names should be first-letter lowercased, with the first letter of each additional word capitalized.
Don't use _ in variable names.
Imports:
Ordered alphabetically
{code}
It seems you also removed the spaces for empty lines, but somehow my eclipse automatically generates those spaces for empty lines.
By the way, did you do it by hand or you auto-generated the changes? If it's automatic, shall we do it one rule by one rule instead of one package by one package?
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by Ning Zhang <nz...@facebook.com>.
I don't know whether checkstyle can check such a case. So we probably don't include it in the automatically checking, but just keep in mind to avoid long statements.
+1 from me.
On Feb 7, 2010, at 10:53 PM, Zheng Shao (JIRA) wrote:
>
> [ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12830852#action_12830852 ]
>
> Zheng Shao commented on HIVE-1123:
> ----------------------------------
>
> I prefer the second format too. Ning, does checkstyle count it as an error or not?
>
> If there are no -1, I will commit these patches on Monday 2/8/2010 afternoon.
>
>
>> Checkstyle fixes
>> ----------------
>>
>> Key: HIVE-1123
>> URL: https://issues.apache.org/jira/browse/HIVE-1123
>> Project: Hadoop Hive
>> Issue Type: Task
>> Reporter: Carl Steinbach
>> Assignee: Carl Steinbach
>> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>>
>>
>> Fix checkstyle errors.
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Zheng Shao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12830852#action_12830852 ]
Zheng Shao commented on HIVE-1123:
----------------------------------
I prefer the second format too. Ning, does checkstyle count it as an error or not?
If there are no -1, I will commit these patches on Monday 2/8/2010 afternoon.
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Carl Steinbach updated HIVE-1123:
---------------------------------
Attachment: HIVE-1123.shims.patch
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.patch, HIVE-1123.common.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.patch, HIVE-1123.serde.patch, HIVE-1123.service.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Ning Zhang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12830233#action_12830233 ]
Ning Zhang commented on HIVE-1123:
----------------------------------
I think all these rules are good to make the coding style consistent, and making the code easier to understand by human once we are used to it.
Sometimes I found our codebase is very "dense" and "compact" and it makes it's hard to understand at the first time. One particular case where I often find hard to understand is a long statement that can usually been broke up by several statements. One example is here:
{code}
Operator op = putOpInsertMap(OperatorFactory.getAndMakeChild(
new groupByDesc(mode, outputColumnNames, groupByKeys, aggregations,
false), new RowSchema(groupByOutputRowResolver.getColumnInfos()),
reduceSinkOperatorInfo), groupByOutputRowResolver);
{code}
I find it is easier to understand if we break it into several short statements with short comments. So when you browse the code, you can only skim the comments and understand what's going on.
It may not be the case that it can be done by style checking, but just want to see what others' opinions on this.
Another more coding style kind of thing is
{code}
groupByKeys.add(new exprNodeColumnDesc(exprInfo.getType(), exprInfo
.getInternalName(), exprInfo.getTabAlias(), exprInfo
.getIsPartitionCol()));
{code}
vs.
{code}
groupByKeys.add(
new exprNodeColumnDesc(
exprInfo.getType(),
exprInfo.getInternalName(),
exprInfo.getTabAlias(),
exprInfo.getIsPartitionCol()));
{code}
Currently some code using the former style. I found it is easier to understand using the second. Is there any convention for this?
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Carl Steinbach updated HIVE-1123:
---------------------------------
Status: Patch Available (was: Open)
Reduces the total number of Checkstyle errors by 50%, from 4361 to 2137.
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.patch, HIVE-1123.common.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.patch, HIVE-1123.serde.patch, HIVE-1123.service.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Zheng Shao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12829924#action_12829924 ]
Zheng Shao commented on HIVE-1123:
----------------------------------
Thanks for the list Carl! This does help a lot!
I talked with a bunch of team members. We will wait till Friday noon for feedbacks and decide what to do with these patches then.
By the way, Carl, is it easy to configure eclipse NOT to produce checkstyle-alarming code?
As far as I know, eclipse leaves spaces in blank lines.
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12829831#action_12829831 ]
Carl Steinbach commented on HIVE-1123:
--------------------------------------
Here is a summary of the changes I made.
For each Checkstyle error message I reference the list of Checkstyle modules that
that signal that error, along with the original error count and error count after applying the
patches.
* Variable 'X' must be private and have accessor methods.
* [VisbilityModifier|http://checkstyle.sourceforge.net/config_design.html#VisibilityModifier]
* 1260 -> 1128
* Name 'X' must match pattern 'X'.
* [ConstantName|http://checkstyle.sourceforge.net/config_naming.html#ConstantName], [LocalFinalVariableName|http://checkstyle.sourceforge.net/config_naming.html#LocalFinalVariableName], [LocalVariableName|http://checkstyle.sourceforge.net/config_naming.html#LocalVariableName], [MemberName|http://checkstyle.sourceforge.net/config_naming.html#MemberName], [MethodName|http://checkstyle.sourceforge.net/config_naming.html#MethodName], [PackageName|http://checkstyle.sourceforge.net/config_naming.html#PackageName], [ParameterName|http://checkstyle.sourceforge.net/config_naming.html#ParameterName], [StaticVariableName|http://checkstyle.sourceforge.net/config_naming.html#StaticVariableName], [TypeName|http://checkstyle.sourceforge.net/config_naming.html#TypeName]
* 569 -> 447
* First sentence should end with a period.
* [JavadocStyle|http://checkstyle.sourceforge.net/config_javadoc.html#JavadocStyle]
* 529 -> 5
* Missing a Javadoc comment.
* [JavadocType|http://checkstyle.sourceforge.net/config_javadoc.html#JavadocType]
* 428 -> 15
* Line is longer than X characters.
* [LineLength|http://checkstyle.sourceforge.net/config_sizes.html#LineLength]
* 310 -> 101
* 'X' modifier out of order with the JLS suggestions.
* [ModifierOrder|http://checkstyle.sourceforge.net/config_modifier.html#ModifierOrder]
* 288 -> 0
* Avoid nested blocks.
* [AvoidNestedBlocks|http://checkstyle.sourceforge.net/config_blocks.html#AvoidNestedBlocks]
* 281 -> 184
* 'X' is followed by whitespace.
* [EmptyForIteratorPad|http://checkstyle.sourceforge.net/config_whitespace.html#EmptyForIteratorPad], [NoWhitespaceAfter|http://checkstyle.sourceforge.net/config_whitespace.html#NoWhitespaceAfter], [ParenPad|http://checkstyle.sourceforge.net/config_whitespace.html#ParenPad], [TypecastParenPad|http://checkstyle.sourceforge.net/config_whitespace.html#TypecastParenPad]
* 190 -> 1
* Redundant 'X' modifier.
* [RedundantModifer|http://checkstyle.sourceforge.net/config_modifier.html#RedundantModifier]
* 172 -> 0
* String literal expressions should be on the left side of an equals comparison.
* [EqualsAvoidNull|http://checkstyle.sourceforge.net/config_coding.html#EqualsAvoidNull]
* 97 -> 93
* Utility classes should not have a public or default constructor.
* [HideUtilityClassConstructor|http://checkstyle.sourceforge.net/config_design.html#HideUtilityClassConstructor]
* 55 -> 4
* Unused import - X
* [UnusedImports|http://checkstyle.sourceforge.net/config_imports.html#UnusedImports]
* 40 -> 5
* Array brackets at illegal position.
* [ArrayTypeStyle|http://checkstyle.sourceforge.net/config_misc.html#ArrayTypeStyle]
* 38 -> 18
* Redundant throws: 'X' is unchecked exception.
* [RedundantThrows|http://checkstyle.sourceforge.net/config_coding.html#RedundantThrows]
* 19 -> 0
* Inner assignments should be avoided.
* [InnerAssignment|http://checkstyle.sourceforge.net/config_coding.html#InnerAssignment]
* 16 -> 14
* 'X' construct must use '{}'s.
* [NeedBraces|http://checkstyle.sourceforge.net/config_blocks.html#NeedBraces]
* 10 -> 1
* Empty statement.
* [EmptyStatement|http://checkstyle.sourceforge.net/config_coding.html#EmptyStatement]
* 10 -> 4
* 'X' is not followed by whitespace.
* [ParenPad|http://checkstyle.sourceforge.net/config_whitespace.html#ParenPad], [TypecastParenPad|http://checkstyle.sourceforge.net/config_whitespace.html#TypecastParenPad], [WhitespaceAfter|http://checkstyle.sourceforge.net/config_whitespace.html#WhitespaceAfter]
* 9 -> 0
* 'X' should be on the same line.
* [RightCurly|http://checkstyle.sourceforge.net/config_blocks.html#RightCurly]
* 8 -> 0
* 'X' is preceded with whitespace.
* [MethodParamPad|http://checkstyle.sourceforge.net/config_whitespace.html#MethodParamPad], [NoWhitespaceBefore|http://checkstyle.sourceforge.net/config_whitespace.html#NoWhitespaceBefore], [ParenPad|http://checkstyle.sourceforge.net/config_whitespace.html#ParenPad]
* 4 -> 0
* Redundant throws: 'X' is subclass of 'X'.
* [RedundantThrows|http://checkstyle.sourceforge.net/config_coding.html#RedundantThrows]
* 3 -> 1
* Class X should be declared as final.
* [FinalClass|http://checkstyle.sourceforge.net/config_design.html#FinalClass]
* 1 -> 0
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12829379#action_12829379 ]
Carl Steinbach commented on HIVE-1123:
--------------------------------------
I favor doing this:
{code:java}
int i;
switch(n) {
case 0:
i = 3;
case 1:
i = 2;
System.out.println(i);
}
{code}
I believe that I have only removed braces in those cases where
the local scope is not necessary. There are a variety of places in
the code where the local scope *is* required, or where the switch
statements are nested. In these cases I have not removed the braces.
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.checkstyle.patch, HIVE-1123.cli.2.patch, HIVE-1123.cli.patch, HIVE-1123.common.2.patch, HIVE-1123.common.patch, HIVE-1123.contrib.2.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.2.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.2.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.2.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.2.patch, HIVE-1123.ql.patch, HIVE-1123.serde.2.patch, HIVE-1123.serde.patch, HIVE-1123.service.2.patch, HIVE-1123.service.patch, HIVE-1123.shims.2.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12828944#action_12828944 ]
Carl Steinbach commented on HIVE-1123:
--------------------------------------
* The Sun coding standard does not specify how to indent "throws" clauses, and the sun_checks.xml configuration that Checkstyle defaults to does not invoke the Indentation module.
* I copied the indentation settings from the Hadoop Checkstyle configuration:
{code}
<module name="Indentation">
<property name="basicOffset" value="2" />
<property name="caseIndent" value="0" />
</module>
{code}
* The configuration options for the Checkstyle Indentation module are described here: http://checkstyle.sourceforge.net/config_misc.html#Indentation
* Note that Checkstyle only gives you one setting, i.e. "basicOffset", so there is no way of saying "indent by 2, unless it's a throws clause, in which case indent by 4 spaces".
I personally prefer indenting throws clauses by 4 spaces in order to make them stand out,
but unfortunately Checkstyle does not make it possible to enforce this and 2 space basic
indentation at the same time. However, I'm willing to compromise since I think the tool is
beneficial over all, and it's important to get the total number of violations down to a low number
so that people will pay attention to it.
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.patch, HIVE-1123.common.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.patch, HIVE-1123.serde.patch, HIVE-1123.service.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HIVE-1123) Checkstyle fixes
Posted by "Zheng Shao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12828960#action_12828960 ]
Zheng Shao commented on HIVE-1123:
----------------------------------
Ok. As long as we have an additional empty line after the "throws" in such cases, I think it's OK.
I also see cases like this. Does this violate the checkstyle indentation rule? How do we know we should have 4 spaces here?
{code}
private void myMethod(Integer a,
Integer b) {
doSomeThing();
}
{code}
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.patch, HIVE-1123.common.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.patch, HIVE-1123.serde.patch, HIVE-1123.service.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HIVE-1123) Checkstyle fixes
Posted by "Carl Steinbach (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HIVE-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Carl Steinbach updated HIVE-1123:
---------------------------------
Attachment: HIVE-1123.contrib.patch
HIVE-1123.common.patch
HIVE-1123.cli.patch
> Checkstyle fixes
> ----------------
>
> Key: HIVE-1123
> URL: https://issues.apache.org/jira/browse/HIVE-1123
> Project: Hadoop Hive
> Issue Type: Task
> Reporter: Carl Steinbach
> Assignee: Carl Steinbach
> Attachments: HIVE-1123.cli.patch, HIVE-1123.common.patch, HIVE-1123.contrib.patch, HIVE-1123.hwi.patch, HIVE-1123.jdbc.patch, HIVE-1123.metastore.patch, HIVE-1123.ql.patch, HIVE-1123.serde.patch, HIVE-1123.service.patch, HIVE-1123.shims.patch
>
>
> Fix checkstyle errors.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.