Anybody looking for a quick patch to contribute?

Richard Bair richard.bair at oracle.com
Wed Mar 13 10:43:51 PDT 2013


Hi Mark,

> I know you guys are busily opening up JavaFX, and I was wondering if there has been any effort to clean up the code? Not trying to critique the code, but some of the earlier code I saw wouldn't have passed minimal checkstyle settings.  

Boy don't I know it! In my multi-step remediation plan, step 1 is to open source, step 2 is to fix the build, step 3 is to document the standards and get check style & find bugs integrated into the build and step 4 is to clean up a whole pile of stuff (much of which is more than cosmetic such as removing classes that are no longer needed and simplifying the code etc).

We have had find bugs and check style around for ages but the build was such that we just didn't get it all properly integrated and used on a regular basis. This is something I want to change.

We also need to document what the standards are on the wiki so that whenever there is a question we've got a document that we can go to. And IDE config files that everybody can use. Even simple things like making sure we use spaces instead of tabs. This has been a long-standing rule but people who start using Eclipse on Windows almost always end up committing change sets with tabs instead of spaces because that is the default setting, so having check style catch this before a push would be great. There are also a lot of idioms in our implementation that good check style / find bugs rules would help make sure we don't push bugs. So for example, I had one yesterday where an EventType suffered from a copy/paste bug which could have been caught by such tools.

> I recently had to delve into LineChart, and there were a number of missing comments on public methods, unnecessary "inheritDocs" tags on overridden methods, funky approaches to iterating through series data that only work within the charts package (can't be used by anyone extending the class), variables like "createSymbols" which are actually anonymous inner classes rather than a simple property.
> 
> Also, do your automated builds run FindBugs or similar tools periodically?  That might also help identify some problem spots before they escape out into the wild.
> 
> Hmm.  Well, I guess I did end up critiquing the code.  When the code review tools come on line it will be easier to give you guys direct feedback on the classes, instead of heckling from the sidelines. :-)

Ya, and any change sets which you want to contribute which clean bits and pieces up (especially low-hanging fruit that is easy to verify doesn't change functionality) would be great.

Richard


More information about the openjfx-dev mailing list