User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:38.0) Gecko/20100101 Thunderbird/38.5.1
I have no strong opinion about this.
What I typically do is fix FindBugs errors while I'm traveling by
plane. This is easier to do when working at the repo head. When I
fix a bug FindBugs bug, I typically document what the bug was in the
code so that it does not get accidentally undone.
I think we should do something fairly lightweight here, such as
create branch, fix things in the branch, commit, push, do a pull
request and then accept the pull request into the master. If the
fixes are significant, then create a bug and discuss the changes in
the bug. If they are not significant, then just do the work with no
bug.
I'm not sure we want a long-lived branch that we are merging from,
my impression is that after we do a pull request, that branch should
not be modified.
Basically, whatever works and is fairly easy. The main thing is fix
warnings and make progress.
For the ErrorCode issue, we should probably create a bug for it that
might not ever get closed. Then, in either the class comment or as
a comment just below the class declaration in the .java file, we
could refer to the bug.
_Christopher
On 1/25/16 3:54 PM, Erwin de Ley wrote:
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)
--
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)