[xwiki-devs] [Proposal] Add SuppressionCommentFilter (CHECKSTYLE:OFF and CHECKSTYLE:ON) to checkstyle.xml
I would like to propose changing checkstyle.xml to allow checkstyle to be enabled and disabled using inline comments. I propose this with some reservation because this can be a slippery slope but I would rather see the problems isolated in the file than the file excluded. Also I think this can be used to enforce checkstyle on changes made to code in big files in the old core. WDYT? Caleb
On 04/15/2010 12:55 PM, Caleb James DeLisle wrote:
I would like to propose changing checkstyle.xml to allow checkstyle to be enabled and disabled using inline comments. I propose this with some reservation because this can be a slippery slope but I would rather see the problems isolated in the file than the file excluded. Also I think this can be used to enforce checkstyle on changes made to code in big files in the old core.
I don't like it very much, since it adds non-code elements in the source file. A source file should contain as little languages as possible, and we already have Java + Javadoc + HTML + English comments, adding checkstyle config would be too much. Can't the error be solved in other ways? What exactly is the problem? -- Sergiu Dumitriu http://purl.org/net/sergiu/
On Apr 15, 2010, at 12:56 PM, Sergiu Dumitriu wrote:
On 04/15/2010 12:55 PM, Caleb James DeLisle wrote:
I would like to propose changing checkstyle.xml to allow checkstyle to be enabled and disabled using inline comments. I propose this with some reservation because this can be a slippery slope but I would rather see the problems isolated in the file than the file excluded. Also I think this can be used to enforce checkstyle on changes made to code in big files in the old core.
I don't like it very much, since it adds non-code elements in the source file. A source file should contain as little languages as possible, and we already have Java + Javadoc + HTML + English comments, adding checkstyle config would be too much.
I agree with Sergiu. Thanks -Vincent
Can't the error be solved in other ways? What exactly is the problem? -- Sergiu Dumitriu http://purl.org/net/sergiu/
Vincent Massol wrote: > On Apr 15, 2010, at 12:56 PM, Sergiu Dumitriu wrote: > >> On 04/15/2010 12:55 PM, Caleb James DeLisle wrote: >>> I would like to propose changing checkstyle.xml to allow checkstyle to be enabled and disabled >>> using inline comments. I propose this with some reservation because this can be a slippery slope >>> but I would rather see the problems isolated in the file than the file excluded. >>> Also I think this can be used to enforce checkstyle on changes made to code in big files in >>> the old core. >> I don't like it very much, since it adds non-code elements in the source >> file. A source file should contain as little languages as possible, and >> we already have Java + Javadoc + HTML + English comments + a few different types of annotations + remnants of aspectj in some places (?). >> , adding checkstyle config would be too much. I'm not sure I would call CHECKSTYLE:ON and CHECKSTYLE:OFF configuration. Also I would want to define a best practice that it only be used in legacy code. > > I agree with Sergiu. > > Thanks > -Vincent > >> Can't the error be solved in other ways? What exactly is the problem? I had planned to use it for fixing most of mailsender api while excluding a method which has too many parameters but changing it would be an api break. I could also see applications when code is added to the old core such as XWiki.java and XWikiDocument.java. I'm not disappointed to see this shot down, introducing tools which make it easier to write dirty code was something I had some reservations about from the beginning. Caleb >> -- >> Sergiu Dumitriu >> http://purl.org/net/sergiu/ > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs >
Well, I agree with all of you but my feeling is that current file exclusion is worse than using these comment in the source code. Since we monitor anyway what is committed, and that we may define correct practice for the usage of these exclusion, I am +1 to use them in old code file (non-component code). Denis On Fri, Apr 16, 2010 at 02:49, Caleb James DeLisle <[email protected] > wrote: > > > Vincent Massol wrote: > > On Apr 15, 2010, at 12:56 PM, Sergiu Dumitriu wrote: > > > >> On 04/15/2010 12:55 PM, Caleb James DeLisle wrote: > >>> I would like to propose changing checkstyle.xml to allow checkstyle to > be enabled and disabled > >>> using inline comments. I propose this with some reservation because > this can be a slippery slope > >>> but I would rather see the problems isolated in the file than the file > excluded. > >>> Also I think this can be used to enforce checkstyle on changes made to > code in big files in > >>> the old core. > >> I don't like it very much, since it adds non-code elements in the source > >> file. A source file should contain as little languages as possible, and > >> we already have Java + Javadoc + HTML + English comments > + a few different types of annotations + remnants of aspectj in some places > (?). > > >> , adding checkstyle config would be too much. > I'm not sure I would call CHECKSTYLE:ON and CHECKSTYLE:OFF configuration. > Also I would want to define > a best practice that it only be used in legacy code. > > > > > I agree with Sergiu. > > > > Thanks > > -Vincent > > > >> Can't the error be solved in other ways? What exactly is the problem? > I had planned to use it for fixing most of mailsender api while excluding a > method which has too many > parameters but changing it would be an api break. I could also see > applications when code is added to > the old core such as XWiki.java and XWikiDocument.java. > > I'm not disappointed to see this shot down, introducing tools which make it > easier to write dirty code > was something I had some reservations about from the beginning. > > Caleb > > >> -- > >> Sergiu Dumitriu > >> http://purl.org/net/sergiu/ > > _______________________________________________ > > devs mailing list > > [email protected] > > http://lists.xwiki.org/mailman/listinfo/devs > > > > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs > -- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
Caleb James DeLisle wrote:
I would like to propose changing checkstyle.xml to allow checkstyle to be enabled and disabled using inline comments. I propose this with some reservation because this can be a slippery slope but I would rather see the problems isolated in the file than the file excluded. Also I think this can be used to enforce checkstyle on changes made to code in big files in the old core.
One problem is that you can easily forget about the pieces of code that have been excluded from checkstyle. Right now classes that are excluded/included from/to checkstyle are listed in the project's pom (those that are excluded should have an explanatory comment, as you suggested). How can you know which classes still have checkstyle errors after they were included in the check list? find/grep on the source tree? Thanks, Marius
WDYT?
Caleb
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
Marius Dumitru Florea wrote:
Caleb James DeLisle wrote:
I would like to propose changing checkstyle.xml to allow checkstyle to be enabled and disabled using inline comments. I propose this with some reservation because this can be a slippery slope but I would rather see the problems isolated in the file than the file excluded. Also I think this can be used to enforce checkstyle on changes made to code in big files in the old core.
One problem is that you can easily forget about the pieces of code that have been excluded from checkstyle. Right now classes that are excluded/included from/to checkstyle are listed in the project's pom (those that are excluded should have an explanatory comment, as you suggested). How can you know which classes still have checkstyle errors after they were included in the check list? find/grep on the source tree? Find-grep is the only way I can think of, I think it's easier to remember whether code is in violation if it has the comments inline. I see a lot of code (in /plugins/) which appears good but in fact is not checked and does have violations.
Caleb
Thanks, Marius
WDYT?
Caleb
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
participants (5)
-
Caleb James DeLisle -
Denis Gervalle -
Marius Dumitru Florea -
Sergiu Dumitriu -
Vincent Massol