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