You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@click.apache.org by "George Stan (JIRA)" <ji...@apache.org> on 2010/05/09 12:24:48 UTC
[jira] Created: (CLK-671) Upgrade to Checkstyle 5.1
Upgrade to Checkstyle 5.1
-------------------------
Key: CLK-671
URL: https://issues.apache.org/jira/browse/CLK-671
Project: Click
Issue Type: Improvement
Reporter: George Stan
Upgrade to Checkstyle 5.1.
It has quite a few fixes and also support for Java 5. Since Click is using Java 5 now this version would be more useful.
Checkstyle 5.1 (but 5.0 too) also contains many checks not used by Click - /build/checkstyle-checks.xml is using just a few of them.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Commented: (CLK-671) Upgrade to Checkstyle 5.1
Posted by Bob Schellink <sa...@gmail.com>.
On 25/05/2010 19:34, George Stan (JIRA) wrote:
> 2. Click catches allot of problems and it's doing allot of checks at runtime, but they consume quite a few cycles.
> What if those checks would be "configurable"/ optional at runtime (so would consume no resources/cycles at all if turned off), but instead they would be as Checkstyle based tasks, that the user can perform at build time?
> In performance intensive installations this could be a fantastic improvement.
Could you share some of the checks you are referring to? When you say fantastic performance gains
are we talking orders of magnitude? Do you have profiled data we could look at to see some of the
bottlenecks?
Kind regards
Bob
Re: [jira] Closed: (CLK-671) Upgrade to Checkstyle 5.1
Posted by "Adrian A." <a....@gmail.com>.
> Oh. I don't think there is a need to run checkstyle on binaries though.
Checkstyle is not "running" on binaries - it only needs them for
implementation simplicity (internal stuff I guess).
> Can we just remove those
> checks that require binary.
Practically most would require the binaries (or might).
This is why they were included in the classpath (by Malcolm?) from the
beginning:
<checkstyle config="build/checkstyle-checks.xml">
<fileset dir="framework/src/org/apache/click/" includes="**/*.java"/>
<fileset dir="extras/src" includes="**/*.java"/>
<fileset dir="mock/src" includes="**/*.java"/>
<classpath refid="classpath.checkstyle"/>
</checkstyle>
and:
<path id="classpath.checkstyle">
<pathelement location="framework/classes"/>
<pathelement location="extras/classes"/>
<pathelement location="mock/classes"/>
.....
usually when working in IDEs (if project paths are configured
correctly), there will always be compiled classes there.
For running on the server, we might add that "depends" property, or
simply inform and let the user do it (the same way it happens with
get-deps too - the user must call it).
Adrian.
Re: [jira] Closed: (CLK-671) Upgrade to Checkstyle 5.1
Posted by Bob Schellink <sa...@gmail.com>.
Oh. I don't think there is a need to run checkstyle on binaries though. Can we just remove those
checks that require binary.
Regards
Bob
On 26/05/2010 16:12, Adrian A. wrote:
>> The changes seems reasonable. Running checkstyle on the codebase
>> throws the following error though.
>> Any ideas?
>>
>> [checkstyle]
>> C:\click-svn\framework\src\org\apache\click\service\TemplateService.java:0:
>> Got an
>> exception - java.lang.RuntimeException: Unable to get class
>> information for @throws tag
>> 'TemplateException'.
> This happens because the new check rules need the class binaries too (to
> be able to perform correctness checking), so checkstyle must be run only
> after the compilation of the sources.
>
> Since our build.xml doesn't have individual "compile" tasks, one needs
> to run build-all first.
> I haven't changed:
> <target name="checkstyle" description="run checkstyle report on Java
> soruce">
>
> to
> <target name="checkstyle" depends="build-all" description="run
> checkstyle report on Java soruce">
>
> because of speed and output reasons (build-all generates lot of output
> on itself)
>
>
Re: [jira] Closed: (CLK-671) Upgrade to Checkstyle 5.1
Posted by "Adrian A." <a....@gmail.com>.
> The changes seems reasonable. Running checkstyle on the codebase throws the following error though.
> Any ideas?
>
> [checkstyle] C:\click-svn\framework\src\org\apache\click\service\TemplateService.java:0: Got an
> exception - java.lang.RuntimeException: Unable to get class information for @throws tag
> 'TemplateException'.
This happens because the new check rules need the class binaries too (to
be able to perform correctness checking), so checkstyle must be run only
after the compilation of the sources.
Since our build.xml doesn't have individual "compile" tasks, one needs
to run build-all first.
I haven't changed:
<target name="checkstyle" description="run checkstyle report on Java
soruce">
to
<target name="checkstyle" depends="build-all" description="run
checkstyle report on Java soruce">
because of speed and output reasons (build-all generates lot of output
on itself)
Re: [jira] Closed: (CLK-671) Upgrade to Checkstyle 5.1
Posted by Bob Schellink <sa...@gmail.com>.
On 26/05/2010 03:14, Adrian A. (JIRA) wrote:
>
> [ https://issues.apache.org/jira/browse/CLK-671?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
>
> Adrian A. closed CLK-671.
> -------------------------
>
> Fix Version/s: 2.3.0
> Resolution: Fixed
The changes seems reasonable. Running checkstyle on the codebase throws the following error though.
Any ideas?
[checkstyle] C:\click-svn\framework\src\org\apache\click\service\TemplateService.java:0: Got an
exception - java.lang.RuntimeException: Unable to get class information for @throws tag
'TemplateException'.
Kind regards
Bob
[jira] Commented: (CLK-671) Upgrade to Checkstyle 5.1
Posted by "George Stan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CLK-671?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12871067#action_12871067 ]
George Stan commented on CLK-671:
---------------------------------
> what exact checks do you have in mind to make use of, besides the actual used ones?
1. Annotations:
http://checkstyle.sourceforge.net/config_annotation.html
- Click is using java 5.
2. Empty Block:
http://checkstyle.sourceforge.net/config_blocks.html#EmptyBlock
3. Headers:
http://checkstyle.sourceforge.net/config_header.html
- to enforce the Apache header from checkstyle, but also to ensure it's consistency.
4. Coding:
http://checkstyle.sourceforge.net/config_coding.html
- to use quite a few from coding (too many to enumerate them here.
5. Duplicate Code:
http://checkstyle.sourceforge.net/config_duplicates.html
6. More from Javadoc:
http://checkstyle.sourceforge.net/config_javadoc.html
- not just the JavadocPackage check
The main point would be that since Click is using Checkstyle, than why not make use of it's entire power (now it looks like it's using only a small percentage of it).
This might be a separate issue:
=========================
I'm not an expert of Checkstyle, but even this would be a fantastic improvement: http://checkstyle.sourceforge.net/extending.html
- to make some custom extended checks
1 - Click specific
2 - Click based webapp specific
3 - to enforce best practices
1. These might be for the Click developers (and Click component developers) to have extra checks not covered by Checkstyle, but to enforce conventions: e.g. the first Control constructor parameter is the name of the control, etc.
2. Click catches allot of problems and it's doing allot of checks at runtime, but they consume quite a few cycles.
What if those checks would be "configurable"/ optional at runtime (so would consume no resources/cycles at all if turned off), but instead they would be as Checkstyle based tasks, that the user can perform at build time?
In performance intensive installations this could be a fantastic improvement.
3. This would allow to enforce some of the best practices described in click docs.
> Upgrade to Checkstyle 5.1
> -------------------------
>
> Key: CLK-671
> URL: https://issues.apache.org/jira/browse/CLK-671
> Project: Click
> Issue Type: Improvement
> Reporter: George Stan
> Assignee: Adrian A.
>
> Upgrade to Checkstyle 5.1.
> It has quite a few fixes and also support for Java 5. Since Click is using Java 5 now this version would be more useful.
> Checkstyle 5.1 (but 5.0 too) also contains many checks not used by Click - /build/checkstyle-checks.xml is using just a few of them.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (CLK-671) Upgrade to Checkstyle 5.1
Posted by "Adrian A. (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CLK-671?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Adrian A. closed CLK-671.
-------------------------
Fix Version/s: 2.3.0
Resolution: Fixed
> Upgrade to Checkstyle 5.1
> -------------------------
>
> Key: CLK-671
> URL: https://issues.apache.org/jira/browse/CLK-671
> Project: Click
> Issue Type: Improvement
> Reporter: George Stan
> Assignee: Adrian A.
> Fix For: 2.3.0
>
>
> Upgrade to Checkstyle 5.1.
> It has quite a few fixes and also support for Java 5. Since Click is using Java 5 now this version would be more useful.
> Checkstyle 5.1 (but 5.0 too) also contains many checks not used by Click - /build/checkstyle-checks.xml is using just a few of them.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CLK-671) Upgrade to Checkstyle 5.1
Posted by "Adrian A. (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CLK-671?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12871251#action_12871251 ]
Adrian A. commented on CLK-671:
-------------------------------
>> what exact checks do you have in mind to make use of, besides the actual used ones?
>1. Annotations:
> http://checkstyle.sourceforge.net/config_annotation.html
Done.
>2. Empty Block:
> http://checkstyle.sourceforge.net/config_blocks.html#EmptyBlock
It's there - line 156, but not active: Click is using many empty blocks right now.
>3. Headers:
> http://checkstyle.sourceforge.net/config_header.html
Done.
>4. Coding:
> http://checkstyle.sourceforge.net/config_coding.html
> - (too many to enumerate them here.
IMHO also too many to add and try them now :). In fact they look so many that it would be an issue on itself to use, configure and tune them.
> 5. Duplicate Code:
> http://checkstyle.sourceforge.net/config_duplicates.html
I would have liked this greatly but it's buggy: it shows the Apache header as duplicate code - so it's not smart at all. If I see right it's simply using a regular expression for this - not something like the IntelliJ inspections.
>6. More from Javadoc:
> http://checkstyle.sourceforge.net/config_javadoc.html
Done.
7. I added a few more that made sense for me. See a diff of the checkstyle file for this.
8. There are a few others commented, because they need more test/tune before enforcing them upon the source code.
> This might be a separate issue .... http://checkstyle.sourceforge.net/extending.html
Interesting concept (thought myself about it while using checkstyle), but please move it to another issue/wiki/discussion because I consider this issue: "upgrade to checkstyle 5.1" as closed :).
> Upgrade to Checkstyle 5.1
> -------------------------
>
> Key: CLK-671
> URL: https://issues.apache.org/jira/browse/CLK-671
> Project: Click
> Issue Type: Improvement
> Reporter: George Stan
> Assignee: Adrian A.
>
> Upgrade to Checkstyle 5.1.
> It has quite a few fixes and also support for Java 5. Since Click is using Java 5 now this version would be more useful.
> Checkstyle 5.1 (but 5.0 too) also contains many checks not used by Click - /build/checkstyle-checks.xml is using just a few of them.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (CLK-671) Upgrade to Checkstyle 5.1
Posted by "Adrian A. (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CLK-671?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Adrian A. reassigned CLK-671:
-----------------------------
Assignee: Adrian A.
> Upgrade to Checkstyle 5.1
> -------------------------
>
> Key: CLK-671
> URL: https://issues.apache.org/jira/browse/CLK-671
> Project: Click
> Issue Type: Improvement
> Reporter: George Stan
> Assignee: Adrian A.
>
> Upgrade to Checkstyle 5.1.
> It has quite a few fixes and also support for Java 5. Since Click is using Java 5 now this version would be more useful.
> Checkstyle 5.1 (but 5.0 too) also contains many checks not used by Click - /build/checkstyle-checks.xml is using just a few of them.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CLK-671) Upgrade to Checkstyle 5.1
Posted by "Adrian A. (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CLK-671?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12870378#action_12870378 ]
Adrian A. commented on CLK-671:
-------------------------------
I updated to checkstyle 5.1, but:
> Checkstyle 5.1 (but 5.0 too) also contains many checks not used by
> Click - /build/checkstyle-checks.xml is using just a few of them.
what exact checks do you have in mind to make use of, besides the actual used ones?
There are just too many checks there, that Click is not using, and we can't just use them all (many don't make sense to Click at all) :).
> Upgrade to Checkstyle 5.1
> -------------------------
>
> Key: CLK-671
> URL: https://issues.apache.org/jira/browse/CLK-671
> Project: Click
> Issue Type: Improvement
> Reporter: George Stan
> Assignee: Adrian A.
>
> Upgrade to Checkstyle 5.1.
> It has quite a few fixes and also support for Java 5. Since Click is using Java 5 now this version would be more useful.
> Checkstyle 5.1 (but 5.0 too) also contains many checks not used by Click - /build/checkstyle-checks.xml is using just a few of them.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.