On 16/06/2008, Matt Benson <[EMAIL PROTECTED]> wrote: > > --- sebb <[EMAIL PROTECTED]> wrote: > > > > On 15/06/2008, Oliver Heger > > <[EMAIL PROTECTED]> wrote: > > > sebb schrieb: > > > > > > > On 14/06/2008, Matt Benson > > <[EMAIL PROTECTED]> wrote: > > > > > > > > > --- Oliver Heger > > <[EMAIL PROTECTED]> wrote: > > > > > > > > > > > +1 > > > > > > > > > > > > Artifacts look very good. I also ran the > > tests for > > > > > > commons configuration > > > > > > with the new version successfully. > > > > > > > > > > > > The only thing that makes me a bit uneasy > > is the > > > > > > findbugs report showing > > > > > > 133 errors. Did you have a look at those? > > > > > > > > > > > > > > > > > > > > > I actually didn't, but I don't see anything in > > there > > > > > that really surprises me. Some false > > positives (e.g. > > > > > String ==), some Serialization issues I knew > > were > > > > > there. It would be nice to attack these for > > another > > > > > release. > > > > > > > > > > > > > Certainly some of them need fixing, e.g. > > > > > > > > Use of non-localized String.toUpperCase() or > > String.toLowerCase > > > > at > > > > > > > > > > > http://people.apache.org/~mbenson/jxpath-1.3-rc3/site/xref/org/apache/commons/jxpath/ri/model/NodePointer.html#549 > > > > and > > > > > > > > > > > http://people.apache.org/~mbenson/jxpath-1.3-rc3/site/xref/org/apache/commons/jxpath/ri/model/dom/DOMNodePointer.html#330 > > > > > > > > These should use something like > > toUpperCase(Locale.ENGLISH). > > > > > > > > Might also be worth adding exclusions for the > > bugs that are false > > > positives... > > > > > > > > > > > I think if the problems were introduced during > > the work on the 1.3 release, > > > they really should be addressed. However if they > > live in the code base for a > > > longer time, they have obviously not caused major > > problems yet, and the > > > strategy to fix them in the next release seems > > reasonable to me. > > > > > > > Good point. > > > > Is this one new? > > > > Dead store to collection in > > > > org.apache.commons.jxpath.ri.model.beans.CollectionPointer.createPath(JXPathContext) > > > > > > http://people.apache.org/~mbenson/jxpath-1.3-rc3/site/xref/org/apache/commons/jxpath/ri/model/beans/CollectionPointer.html#125 > > > > The code certainly looks odd ... > > > > > Not new, and from the POV of understanding what the > code there does (grows the underlying collection to a > size such that the index is valid) doesn't appear > problematic.
What's confusing is that the collection variable is assigned, but not used. Perhaps remove the assignment to make it clearer. > > > (most of the other dead store reports seem to be > > FPs) > > > > > > If getMessage() is heavily used, then this one > > should be fixed: > > > > Method > > > org.apache.commons.jxpath.ri.parser.ParseException.getMessage() > > concatenates strings using + in a loop > > > This is generated code. It wouldn't hurt to fix it, > since the JXPath parser code is generated and then > saved, but in theory if we ever regenerated the parser > from the JavaCC grammar we'd be starting from scratch > to add back any improvements made. It being the case > that the rightness of making changes to generated code > is in doubt, I certainly wouldn't think the release > should be held up because of it. Sorry, did not notice that was generated code. There were plenty of other complaints about the generated code which I did ignore. > So far the only issue I've seen that I would think is > terror-worthy is Phil's discovery of my having left in > some [io] references when I (relatively) recently > cloned its build instructions for a much-needed > update. He, however, has given his +1. > > In any event, all these issues should be addressed at > some point, so I'll go ahead and work on these in > trunk. > That's fine by me. > > -Matt > > > > > > > Oliver > > > > > > > > > > > > > > > > > > > > > Does your +1 still stand? > > > > > > > > > > > > > > > -Matt > > > > > > > > > > > > > > > > Oliver > > > > > > > > > > > > Matt Benson schrieb: > > > > > > > Thanks to anyone who reported issues with > > the > > > > > > previous > > > > > > > two release candidates, and especially to > > those > > > > > > who > > > > > > > helped resolve them. > > > > > > > > > > > > > > The artifacts are here: > > > > > > > > > http://people.apache.org/~mbenson/jxpath-1.3-rc3/ > > > > > > > > > > > > > > The tag is here: > > > > > > > > > > > > > > > > > > > > > > > > http://svn.apache.org/viewvc/commons/proper/jxpath/tags/JXPATH_1_3_RC3/ > > > > > > > > > > > > > > Site: > > > > > > > > > > > > > > > > > > > > > http://people.apache.org/~mbenson/jxpath-1.3-rc3/site > > > > > > > > > > > > > > Clirr Report (compared to 1.2; one-shot > > not > > > > > > working w/ > > > > > > > M2) > > > > > > > > > > > > > > > > > > > > > > > > http://people.apache.org/~mbenson/jxpath-1.3-rc3/clirr-report.txt > > > > > > > > > > > > > > I'd be grateful if you can make time to > > check the > > > > > > > artifacts and cast your vote, which will > > be open > > > > > > at > > > > > > > least until Friday, June 20. > > > > > > > > > > > > > > Thanks, > > > > > > > Matt > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > > > > > To unsubscribe, e-mail: > > > > > > [EMAIL PROTECTED] > > > > > > > For additional commands, e-mail: > > > > > > [EMAIL PROTECTED] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > > > > To unsubscribe, e-mail: > > > > > > [EMAIL PROTECTED] > > > > > > For additional commands, e-mail: > > > > > > [EMAIL PROTECTED] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > > > To unsubscribe, e-mail: > > > [EMAIL PROTECTED] > > > > > For additional commands, e-mail: > > [EMAIL PROTECTED] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > > To unsubscribe, e-mail: > > > [EMAIL PROTECTED] > > > > For additional commands, e-mail: > > [EMAIL PROTECTED] > > > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: > > [EMAIL PROTECTED] > > > For additional commands, e-mail: > > [EMAIL PROTECTED] > > > > > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: > > [EMAIL PROTECTED] > > For additional commands, e-mail: > > [EMAIL PROTECTED] > > > > > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
