User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1
Yep. But Findbugs will be a great help indeed to discover coding
issues.
Now, how do we work on fixing code (or adding docs about why things
are as they are)?
Do we use a long-living branch "fix stuff found by FindBugs" from
where we merge on a regular basis?
Or do we work straight-away on master for small changes? (I think at
this stage that would be ok)
Now that we have these numerical indicators, we need to get the
count of errors/warnings down! ;-)
erwin
Op 26/01/2016 om 00:36 schreef
Christopher Brooks:
I think we can ignore the ErrorCode issue for the time being and
move on.
_C
On 1/25/16 3:31 PM, erwindl0 wrote:
ErrorCode is a quite particular
thing that benefits from being "enumerator-like".
It's used to manage an application-wide registry of error
codes, severities, descriptions, formatted error logging (to
allow automated parsing of log files) etc.
I.e. they're not just numeric constants or so.
The goal is also to allow extension modules to add their own
instances, and an easy approach for this is to subclass it and
to add extra instances there.
I like the enum-like syntax, as this makes it easy to use.
Personally I don't know how to get the same set of features
with interfaces...
erwin
Op 25/01/2016 om 23:16 schreef Christopher Brooks:
If an interface would work for ErrorCode, then that would be
good. However, I'd have to look into it.
FindBugs is picky about some odd things, but I've definitely
found bugs that would be hard to find otherwise. Coverity
Scan is also available to us and finds even more bugs.
FindBugs is sometimes wrong. When it is, I add a comment
about why I think it is wrong to the code.
Adding FindBugs was easy, I just added the following to the
<plugins>...</plugins> section of the toplevel
pom.xml
Running "mvn findbugs:gui" brings up the FindBugs gui for each
package.
Then I configured Hudson to generate FindBugs reports for and
to look for **/findbugsXml.xml files.
I'll work on generating JavaDoc files next, then running
doccheck to find problems with the JavaDoc.
_C
On 1/25/16 12:46 PM, Jay Jay
Billings wrote:
W.r.t to ErrorCode, personally I always use an
interface for this type of thing. Gets around this naming
problem and decreases branching on the flip side too.
I need to get ICE building on the CBI and hook
FindBugs up to it. We use SonarQube internally but I'd
interested in seeing what FindBugs... er... finds.
I've checked the
report and there are definitely a series of warnings
that I want to have resolved.
Good idea to add it in our build!
cheers
erwin
Op 25/01/2016 om 21:26 schreef Erwin de Ley:
Hi Christopher,
I agree that we should follow all kinds of good
practices, and FindBugs probably has a valid set of
them in its default rules.
But... for the ErrorCode naming I'm a bit afraid to
end up with many long names for each extension of
the base ErrorCode pseudo-enum.
And that this could reduce readability of code i.o.
improving it.
E.g. for a typical usage of an extended ErrorCode
such as org.eclipse.triquetrum.processing.ErrorCode
: in DefaultTaskProcessingBroker there is a line
like :
futResult.completeExceptionally(new
ProcessingException(ErrorCode.TASK_UNHANDLED, "No
service found for " + task, null));
I think this is quite readable, and better than e.g.
futResult.completeExceptionally(new
ProcessingException(ProcessingErrorCode.TASK_UNHANDLED,
"No service found for " + task, null));
We could maybe start using static imports if we go
for the specialized names? Then such code would
become
futResult.completeExceptionally(new
ProcessingException(TASK_UNHANDLED, "No service
found for " + task, null));
Personally I'm not fond of static imports, but I see
many of our younger developers using them, so maybe
I'm just a bit old-fashioned ;-)
What do you think?
regards
erwin
Op 25/01/2016 om 20:44 schreef Christopher
Brooks:
ErrorCode.java:26,
NM_SAME_SIMPLE_NAME_AS_SUPERCLASS,
Priority: High
The class name
org.eclipse.triquetrum.validation.ErrorCode
shadows the simple name of the
superclass
org.eclipse.triquetrum.ErrorCode
This class has a simple name that is
identical to that of its superclass,
except that its superclass is in a
different package (e.g., alpha.Foo
extends beta.Foo). This
can be exceptionally confusing, create
lots of situations in which you have
to look at import statements to
resolve references and creates many
opportunities to accidently define
methods that do not override methods
in their superclasses.
One fix would be to rename
org.eclipse.triquetrum.validation.ErrorCode to
org.eclipse.triquetrum.validation.ErrorCodeValidation
or something.
Or, we could ignore it.
I've found that fixing as many FindBugs warnings
as possible is really helpful because it makes the
actual bugs standout. The downside of fixing
these bugs is that some of the changes are not
natural.
What are your thoughts?
_Christopher
--
Christopher Brooks, PMP University of California
Academic Program Manager & Software Engineer US Mail: 337 Cory Hall
CHESS/iCyPhy/Ptolemy/TerraSwarm Berkeley, CA 94720-1774
cxh@xxxxxxxxxxxxxxxxx, 707.332.0670 (Office: 545Q Cory)
--
Christopher Brooks, PMP University of California
Academic Program Manager & Software Engineer US Mail: 337 Cory Hall
CHESS/iCyPhy/Ptolemy/TerraSwarm Berkeley, CA 94720-1774
cxh@xxxxxxxxxxxxxxxxx, 707.332.0670 (Office: 545Q Cory)
--
Christopher Brooks, PMP University of California
Academic Program Manager & Software Engineer US Mail: 337 Cory Hall
CHESS/iCyPhy/Ptolemy/TerraSwarm Berkeley, CA 94720-1774
cxh@xxxxxxxxxxxxxxxxx, 707.332.0670 (Office: 545Q Cory)