Collectible Card Game Headquarters

Checkstyle is based on the Sun Code Conventions for Java
http://www.oracle.com/technetwork/java/codeconvtoc-136057.html
I created a custom file in src/main/config/forge_checks.xml to reduce the checks to a more manageable size.

Braids wrote:we've not had coding standards before, so when asked about them, CheckStyle was the only reference i have. it's obviously making you unhappy, so let's fix it.

are there particular things you like that CheckStyle does not?

do you want to get rid of it entirely? i want this discussion to feel less dictatorial and more democratic.

I mostly want to make sure there's understanding when there are Project wide changes occurring. There have been a lot of these in the last few months, and where some of the others were mostly one time changes (setting up Git/new SVN); this situation will affect the day-to-day work done on Forge, so I want to make sure it's clear what's going on. As I mentioned above, I don't know all the things that Checkstyle is checking, so right now I don't have any specific complaints about it. But without a more information about what it's looking for, I can't attempt to meet the guidelines, nor argue that a certain guideline is unnecessary.

I'm not voting for getting rid of the system, I'm sure it will be useful. It's the lack of knowledge regarding the system that is bugging me the most.

I installed the CheckStyle plugin. I was currious to see what sort of problems that it would find. I can right click on the project and there is a CheckStyle menu item with several different options.

It appears that I need a valid check configuration.

friarsol wrote:

Braids wrote:we've not had coding standards before, so when asked about them, CheckStyle was the only reference i have. it's obviously making you unhappy, so let's fix it.

are there particular things you like that CheckStyle does not?

do you want to get rid of it entirely? i want this discussion to feel less dictatorial and more democratic.

I mostly want to make sure there's understanding when there are Project wide changes occurring.

ok. are new topics in the Developer's Corner a sufficient means of communication?

friarsol wrote:As I mentioned above, I don't know all the things that Checkstyle is checking, so right now I don't have any specific complaints about it. But without a more information about what it's looking for, I can't attempt to meet the guidelines, nor argue that a certain guideline is unnecessary.

there is an old report at http://cardforge.googlecode.com/svn/site/checkstyle.html. you can subscribe to this topic to see if someone knows of a more recent one.

also, if you use Eclipse with the CheckStyle plugin, it will start flagging your CheckStyle violations as pseudo-errors in any file that you have open. if your code compiles, you can still test it, even though it has these "errors".

unfortunately, this sort of error flagging makes it really, really tempting to fix all the CheckStyle errors at once, creating the problem that spurred Max mtg into creating this topic in the first place.

friarsol wrote:I'm not voting for getting rid of the system, I'm sure it will be useful. It's the lack of knowledge regarding the system that is bugging me the most.

the truth is, there's nothing out there that prevents you from writing and committing code that violates CheckStyle {or FindBugs or PMD}, even profusely. these three tools each upload a report when certain Maven commands are run. i think that may be why it was introduced without fanfare. it didn't really break anything. i just picked it up and said, oh, a coding standard, we actually have one? and ran with it.

Chris H. wrote:I installed the CheckStyle plugin. I was currious to see what sort of problems that it would find. I can right click on the project and there is a CheckStyle menu item with several different options.

It appears that I need a valid check configuration.

i just committed a fix to rev 9803.

Sorry to open an "old" discussion, but this thread sort of tailed off without any resolution.

It seems people are generally in favor of consistently formatted code, and have made several non-conflicting suggestions about guidelines. With the great organization/standards that Maven and Mantis bring to the table, it seems a logical/professional move to use an agreed set of checkstyles, and mold the code to fit over time.

Anyway, should I be making an effort to follow the current checkstyle, or is it going to change in the future? Incidentally, I'd be happy to do some time correcting formatting "errors" should there be any guidelines decided upon.

Doublestrike wrote:Sorry to open an "old" discussion, but this thread sort of tailed off without any resolution.

It seems people are generally in favor of consistently formatted code, and have made several non-conflicting suggestions about guidelines. With the great organization/standards that Maven and Mantis bring to the table, it seems a logical/professional move to use an agreed set of checkstyles, and mold the code to fit over time.

Anyway, should I be making an effort to follow the current checkstyle, or is it going to change in the future? Incidentally, I'd be happy to do some time correcting formatting "errors" should there be any guidelines decided upon.

i exspect this thread's Ressurection any time someone has issues with the CheckStyle or Eclipse settings for style.

mantis has little to do with style. it is more about reducing bug reports and feature requests in the forums. last i heard, the plan was to retire the Current Known Bugs List {and possibly other topics} once we had worked out the bugs and inconsistencies between the internal bug reporter and the english guide for reporting bugs. to maintain comfort for those using the forum, they can post bug reports for the latest beta release in the announcement thread for that release. this would prevent enormous threads from forming.

maven's role in this is to generate reports for public ridicule.

some people have been moulding the code to fit over time, yes. the checkstyle plugin is optional, and it still allows violations. it installed, it flags them as errors. but they act more like warnings.

for new files, please follow checkstyle for the entire file. i doubt the style config will change too much in the future. if it does, it will probably be less stringent, so your efforts are not wasted.

when you edit a block of code somewhere else, or if you rename a file, follow checkstyle for the lines you are editing. please resist the urge to fix all formatting in the file. after you've tested and committed the file(s), you can then go back and fix checkstyle errors in a separate commit. (i just committed a setting change to rev 10014 that makes this easier. {previously it was ignoring certain files, including freshly committed ones. you still have to open a file in the editor to see its checkstyle errors.}}

i would suggest against changing the style of any other files at this time.

as of rev 9980, if you use Eclipse, the default comments and code it generates {like when you tell Eclipse to create a class or a method} are more in line with checkstyle.

many times i've noticed the checkstyle plugin gives false errors about files not ending in newline. also, it doesn't like many the classes in src/test/java/, because they lack package-info. if your file has one or two newlines at the end, you can ignore these so called errors.

Personally I don't like checkstyle marking warnings as errors, because it floods the really-need-fixing compile errors list.

My original intent of raising this topic was to restrict autoformatter use on old code with options varying from developer to developer.