You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by "Tom White (JIRA)" <ji...@apache.org> on 2007/02/28 21:38:50 UTC
[jira] Created: (HADOOP-1051) Add checkstyle target to ant build
file
Add checkstyle target to ant build file
---------------------------------------
Key: HADOOP-1051
URL: https://issues.apache.org/jira/browse/HADOOP-1051
Project: Hadoop
Issue Type: Test
Components: build, test
Affects Versions: 0.11.2
Reporter: Tom White
Assigned To: Tom White
As discussed in HADOOP-948, add a target to allow people to run style checks on the codebase.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Commented: (HADOOP-1051) Add checkstyle target to ant build file
Posted by Nigel Daley <nd...@yahoo-inc.com>.
I'll fix the patch process to omit *.html and *.htm attachments when
choosing a patch file.
On Feb 28, 2007, at 1:11 PM, Hadoop QA (JIRA) wrote:
>
> [ https://issues.apache.org/jira/browse/HADOOP-1051?
> page=com.atlassian.jira.plugin.system.issuetabpanels:comment-
> tabpanel#action_12476714 ]
>
> Hadoop QA commented on HADOOP-1051:
> -----------------------------------
>
> -1, because the patch command could not apply the latest attachment
> http://issues.apache.org/jira/secure/attachment/12352266/checkstyle-
> errors.html as a patch to trunk revision http://svn.apache.org/
> repos/asf/lucene/hadoop/trunk/512944. Please note that this message
> is automatically generated and may represent a problem with the
> automation system and not the patch. Results are at http://
> lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch
>
>> Add checkstyle target to ant build file
>> ---------------------------------------
>>
>> Key: HADOOP-1051
>> URL: https://issues.apache.org/jira/browse/
>> HADOOP-1051
>> Project: Hadoop
>> Issue Type: Test
>> Components: build, test
>> Affects Versions: 0.11.2
>> Reporter: Tom White
>> Assigned To: Tom White
>> Attachments: checkstyle-errors.html, checkstyle.patch
>>
>>
>> As discussed in HADOOP-948, add a target to allow people to run
>> style checks on the codebase.
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>
[jira] Commented: (HADOOP-1051) Add checkstyle target to ant build
file
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12476714 ]
Hadoop QA commented on HADOOP-1051:
-----------------------------------
-1, because the patch command could not apply the latest attachment http://issues.apache.org/jira/secure/attachment/12352266/checkstyle-errors.html as a patch to trunk revision http://svn.apache.org/repos/asf/lucene/hadoop/trunk/512944. Please note that this message is automatically generated and may represent a problem with the automation system and not the patch. Results are at http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch
> Add checkstyle target to ant build file
> ---------------------------------------
>
> Key: HADOOP-1051
> URL: https://issues.apache.org/jira/browse/HADOOP-1051
> Project: Hadoop
> Issue Type: Test
> Components: build, test
> Affects Versions: 0.11.2
> Reporter: Tom White
> Assigned To: Tom White
> Attachments: checkstyle-errors.html, checkstyle.patch
>
>
> As discussed in HADOOP-948, add a target to allow people to run style checks on the codebase.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1051) Add checkstyle target to ant build
file
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477389 ]
Doug Cutting commented on HADOOP-1051:
--------------------------------------
> Is it worth excluding attached files that don't have an ASF license granted too?
Or, if they're patches, adding -1 for that reason?
Also, rather than checking for other file types, couldn't we instead check that something's indeed a patch file, and ignore all non-patch attachments? The unix 'file' command does a good job of identifying patches as 'RCS/CVS diff output text'.
> Add checkstyle target to ant build file
> ---------------------------------------
>
> Key: HADOOP-1051
> URL: https://issues.apache.org/jira/browse/HADOOP-1051
> Project: Hadoop
> Issue Type: Test
> Components: build, test
> Affects Versions: 0.11.2
> Reporter: Tom White
> Assigned To: Tom White
> Attachments: checkstyle-errors.html, checkstyle-v2.patch, checkstyle.patch
>
>
> As discussed in HADOOP-948, add a target to allow people to run style checks on the codebase.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1051) Add checkstyle target to ant build
file
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477079 ]
Doug Cutting commented on HADOOP-1051:
--------------------------------------
+0
It might be useful to add this. A patch should ideally not increase the number of style warnings. But, first, we'll need to agree on the default settings, which will be contentious. For example, I find the following warnings spurious:
'cast' is not followed by whitespace.
'+' should be on a new line.
'||' should be on a new line.
'1024' is a magic number.
'0xffff' is a magic number.
Using the '.*' form of import should be avoided - org.apache.commons.logging.*.
I'm even okay with if's that don't have braces when there's no 'else' clause. Indentation tells the story there quite well and it's not a source of errors or misunderstandings. But I suspect there are those who will argue with that.
On the other hand, I'm bothered by lines over 80 columns and non-2-space indentation (the testing of which you've disabled) while many others are apparently not. However I've managed to collaborate on projects with such people for many years, without any serious problems.
Reasonable people differ about these things. I fear we could waste a lot of time bickering about the standard style definition at the expense of getting things done. Selective enforcement by committers, with all its pitfalls, may be a more pragmatic route.
> Add checkstyle target to ant build file
> ---------------------------------------
>
> Key: HADOOP-1051
> URL: https://issues.apache.org/jira/browse/HADOOP-1051
> Project: Hadoop
> Issue Type: Test
> Components: build, test
> Affects Versions: 0.11.2
> Reporter: Tom White
> Assigned To: Tom White
> Attachments: checkstyle-errors.html, checkstyle.patch
>
>
> As discussed in HADOOP-948, add a target to allow people to run style checks on the codebase.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-1051) Add checkstyle target to ant build
file
Posted by "Tom White (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Tom White updated HADOOP-1051:
------------------------------
Attachment: checkstyle-v2.patch
I too don't want to waste time arguing over which set of checks to enforce. Instead I would suggest a lowest common denominator set of checks which are broadly uncontentious, and which committers can run against patches at their own discretion to check that they are basically OK.
This second patch remove's Doug's spurious warnings and adds David's change for indentation checking.
I take the point about reasonable people differing on these issues, which is why I would not integrate this into the build tool and instead give the committers the final say.
> Add checkstyle target to ant build file
> ---------------------------------------
>
> Key: HADOOP-1051
> URL: https://issues.apache.org/jira/browse/HADOOP-1051
> Project: Hadoop
> Issue Type: Test
> Components: build, test
> Affects Versions: 0.11.2
> Reporter: Tom White
> Assigned To: Tom White
> Attachments: checkstyle-errors.html, checkstyle-v2.patch, checkstyle.patch
>
>
> As discussed in HADOOP-948, add a target to allow people to run style checks on the codebase.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1051) Add checkstyle target to ant build
file
Posted by "Tom White (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477302 ]
Tom White commented on HADOOP-1051:
-----------------------------------
> I'll fix the patch process to omit *.html and *.htm attachments when
> choosing a patch file.
Is it worth excluding attached files that don't have an ASF license granted too?
> Add checkstyle target to ant build file
> ---------------------------------------
>
> Key: HADOOP-1051
> URL: https://issues.apache.org/jira/browse/HADOOP-1051
> Project: Hadoop
> Issue Type: Test
> Components: build, test
> Affects Versions: 0.11.2
> Reporter: Tom White
> Assigned To: Tom White
> Attachments: checkstyle-errors.html, checkstyle-v2.patch, checkstyle.patch
>
>
> As discussed in HADOOP-948, add a target to allow people to run style checks on the codebase.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-1051) Add checkstyle target to ant build
file
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Doug Cutting updated HADOOP-1051:
---------------------------------
Resolution: Fixed
Fix Version/s: 0.12.0
Status: Resolved (was: Patch Available)
These rules look fine to me, so I committed this. Thanks, Tom!
(I also made it so that subversion will ignore files named 'checkstyle*' in the lib directory, so that we won't accidentally check this LGPL'd jar in.)
> Add checkstyle target to ant build file
> ---------------------------------------
>
> Key: HADOOP-1051
> URL: https://issues.apache.org/jira/browse/HADOOP-1051
> Project: Hadoop
> Issue Type: Test
> Components: build, test
> Affects Versions: 0.11.2
> Reporter: Tom White
> Assigned To: Tom White
> Fix For: 0.12.0
>
> Attachments: checkstyle-errors.html, checkstyle-v2.patch, checkstyle.patch
>
>
> As discussed in HADOOP-948, add a target to allow people to run style checks on the codebase.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-1051) Add checkstyle target to ant build
file
Posted by "Tom White (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Tom White updated HADOOP-1051:
------------------------------
Status: Patch Available (was: Open)
> Add checkstyle target to ant build file
> ---------------------------------------
>
> Key: HADOOP-1051
> URL: https://issues.apache.org/jira/browse/HADOOP-1051
> Project: Hadoop
> Issue Type: Test
> Components: build, test
> Affects Versions: 0.11.2
> Reporter: Tom White
> Assigned To: Tom White
> Attachments: checkstyle-errors.html, checkstyle.patch
>
>
> As discussed in HADOOP-948, add a target to allow people to run style checks on the codebase.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1051) Add checkstyle target to ant build
file
Posted by "David Bowen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477081 ]
David Bowen commented on HADOOP-1051:
-------------------------------------
+1. I like the idea of doing this. Following code conventions makes code easier to read and to modify.
It would be good to get the indentation consistent. I don't like the 2-space convention (I much prefer 4-space) but I'd rather have 2-space everywhere than the current situation where the spacing is inconsistent.
Can the checkstyle jar be checked into the lib directory so that people don't have to download it?
> Add checkstyle target to ant build file
> ---------------------------------------
>
> Key: HADOOP-1051
> URL: https://issues.apache.org/jira/browse/HADOOP-1051
> Project: Hadoop
> Issue Type: Test
> Components: build, test
> Affects Versions: 0.11.2
> Reporter: Tom White
> Assigned To: Tom White
> Attachments: checkstyle-errors.html, checkstyle.patch
>
>
> As discussed in HADOOP-948, add a target to allow people to run style checks on the codebase.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-1051) Add checkstyle target to ant build
file
Posted by "Tom White (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Tom White updated HADOOP-1051:
------------------------------
Attachment: checkstyle-errors.html
Here is a sample report for the codebase produced by the checkstyle target.
> Add checkstyle target to ant build file
> ---------------------------------------
>
> Key: HADOOP-1051
> URL: https://issues.apache.org/jira/browse/HADOOP-1051
> Project: Hadoop
> Issue Type: Test
> Components: build, test
> Affects Versions: 0.11.2
> Reporter: Tom White
> Assigned To: Tom White
> Attachments: checkstyle-errors.html, checkstyle.patch
>
>
> As discussed in HADOOP-948, add a target to allow people to run style checks on the codebase.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1051) Add checkstyle target to ant build
file
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477084 ]
Doug Cutting commented on HADOOP-1051:
--------------------------------------
> Can the checkstyle jar be checked into the lib directory so that people don't have to download it?
No. It's available under the LGPL, and Apache policy prohibts redistribution of LGPL'd stuff.
http://people.apache.org/~cliffs/3party.html
> Add checkstyle target to ant build file
> ---------------------------------------
>
> Key: HADOOP-1051
> URL: https://issues.apache.org/jira/browse/HADOOP-1051
> Project: Hadoop
> Issue Type: Test
> Components: build, test
> Affects Versions: 0.11.2
> Reporter: Tom White
> Assigned To: Tom White
> Attachments: checkstyle-errors.html, checkstyle.patch
>
>
> As discussed in HADOOP-948, add a target to allow people to run style checks on the codebase.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1051) Add checkstyle target to ant build
file
Posted by "David Bowen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477133 ]
David Bowen commented on HADOOP-1051:
-------------------------------------
Here's the config change to add indentation checking:
<module name="Indentation">
<property name="basicOffset" value="2" />
<property name="caseIndent" value="2" />
</module>
(The default sun_checks.xml does not check indentation.)
> Add checkstyle target to ant build file
> ---------------------------------------
>
> Key: HADOOP-1051
> URL: https://issues.apache.org/jira/browse/HADOOP-1051
> Project: Hadoop
> Issue Type: Test
> Components: build, test
> Affects Versions: 0.11.2
> Reporter: Tom White
> Assigned To: Tom White
> Attachments: checkstyle-errors.html, checkstyle.patch
>
>
> As discussed in HADOOP-948, add a target to allow people to run style checks on the codebase.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-1051) Add checkstyle target to ant build
file
Posted by "Tom White (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Tom White updated HADOOP-1051:
------------------------------
Attachment: checkstyle.patch
This patch adds a "checkstyle" target that is independent of other targets. We might want to add it to the "test" target in the future, but for now the idea is that contributors can run it to see if their changes introduce style errors. Similarly, committers can run it to see if patches increase the number of warnings.
This is a candidate for adding to the Hudson build too.
The current set of styles that is checked is a cutdown copy of the Sun coding conventions. (It is cutdown since there were a number of checks to do with whitespace that had hundreds of violations, so I removed them for the moment.) We can add or remove checks over time.
> Add checkstyle target to ant build file
> ---------------------------------------
>
> Key: HADOOP-1051
> URL: https://issues.apache.org/jira/browse/HADOOP-1051
> Project: Hadoop
> Issue Type: Test
> Components: build, test
> Affects Versions: 0.11.2
> Reporter: Tom White
> Assigned To: Tom White
> Attachments: checkstyle.patch
>
>
> As discussed in HADOOP-948, add a target to allow people to run style checks on the codebase.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1051) Add checkstyle target to ant build
file
Posted by "Tom White (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12476705 ]
Tom White commented on HADOOP-1051:
-----------------------------------
I forgot to mention that you need to download checkstyle 4.3 and install the "all" jar in the lib directory before using the new target.
> Add checkstyle target to ant build file
> ---------------------------------------
>
> Key: HADOOP-1051
> URL: https://issues.apache.org/jira/browse/HADOOP-1051
> Project: Hadoop
> Issue Type: Test
> Components: build, test
> Affects Versions: 0.11.2
> Reporter: Tom White
> Assigned To: Tom White
> Attachments: checkstyle-errors.html, checkstyle.patch
>
>
> As discussed in HADOOP-948, add a target to allow people to run style checks on the codebase.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.