I'm a fan of fixing the stuff reported by FindBugs. However, there are some things to be careful of.

Too much code change can sometimes have the reverse effect that was intended. In certain cases '2 bugs' could be working together to cause the end code to actually function. So if you only 'fix' one bug, you could introduce a problem that did not exist before.

This is a real annoyance to people who are simply trying to apply the latest patch. So, I would be very careful how much you do in the 3.1.x branch. I believe trunk is a lot more forgiving for these types of changes.

Another thing you can do, is to make small changes like the one you've attached to the jira so far. That's a nice small, well controlled patch.

Also, if you really want to work on improving the 3.1.x branch, make sure you include both 3.1.x branch *AND* trunk patches.

---- Cris J H

Ernst-Jan Verhoeven wrote:
Hi all,

I'm a developer for the University of Amsterdam and we are planning to run uPortal 3.1 in production in a few months. Of course we like the system to be as stable as possible, and as me and my colleagues are huge fans of automated code-review tools we ran the automated (byte-)code review tool 'Findbugs' on uPortal.

Not all issue-categories reported by Findbugs are equally easy or rewarding to fix. I picked the '(possible) null pointer derefences' category as it usually contains issues that are both easy and rewarding to fix and spent an afternoon trying to fix these issues. I have included my efforts in a Jira report:

http://www.ja-sig.org/issues/browse/UP-2443

This was more of an exercise to get a feeling of the maintainability of the code, but we intend to do more fixes like this.

Other nice candidates seem e.g.

'method may fail to close database resource' (33 reported Findbugs-issues)
'Method may fail to clean up stream or resource' (40 reported Findbugs-issues)

although in my experience (and/or opinion) Findbugs usually reports a very low number of false positives; so it may not be that useful to 'zoom in' on specific categories.

How do others feel about this 'bottom-up' way of fixing possible issues in the uPortal code-base?

-Ernst-Jan Verhoeven



--
You are currently subscribed to [email protected] as: 
[email protected]
To unsubscribe, change settings or access archives, see 
http://www.ja-sig.org/wiki/display/JSG/uportal-dev

Reply via email to