commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [VOTE] Release JXPath 1.3 based on RC3
Date Mon, 16 Jun 2008 15:44:18 GMT
On 16/06/2008, Matt Benson <gudnabrsam@yahoo.com> wrote:
>
>  --- sebb <sebbaz@gmail.com> wrote:
>
>
> > On 15/06/2008, Oliver Heger
>  > <oliver.heger@oliver-heger.de> wrote:
>  > > sebb schrieb:
>  > >
>  > > > On 14/06/2008, Matt Benson
>  > <gudnabrsam@yahoo.com> wrote:
>  > > >
>  > > > >  --- Oliver Heger
>  > <oliver.heger@oliver-heger.de> 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:
>  > > > >  > dev-unsubscribe@commons.apache.org
>  > > > >  > > For additional commands, e-mail:
>  > > > >  > dev-help@commons.apache.org
>  > > > >  > >
>  > > > >  >
>  > > > >  >
>  > > > >  >
>  > > > >
>  > >
>  >
>  ---------------------------------------------------------------------
>  > > > >  > To unsubscribe, e-mail:
>  > > > >  > dev-unsubscribe@commons.apache.org
>  > > > >  > For additional commands, e-mail:
>  > > > >  > dev-help@commons.apache.org
>  > > > >  >
>  > > > >  >
>  > > > >
>  > > > >
>  > > > >
>  > > > >
>  > > > >
>  > > > >
>  > >
>  >
>  ---------------------------------------------------------------------
>  > > > >  To unsubscribe, e-mail:
>  > > dev-unsubscribe@commons.apache.org
>  > > > >  For additional commands, e-mail:
>  > dev-help@commons.apache.org
>  > > > >
>  > > > >
>  > > > >
>  > > >
>  > > >
>  > >
>  >
>  ---------------------------------------------------------------------
>  > > > To unsubscribe, e-mail:
>  > > dev-unsubscribe@commons.apache.org
>  > > > For additional commands, e-mail:
>  > dev-help@commons.apache.org
>  > > >
>  > > >
>  > >
>  > >
>  > >
>  >
>  ---------------------------------------------------------------------
>  > >  To unsubscribe, e-mail:
>  > dev-unsubscribe@commons.apache.org
>  > >  For additional commands, e-mail:
>  > dev-help@commons.apache.org
>  > >
>  > >
>  >
>  >
>  ---------------------------------------------------------------------
>  > To unsubscribe, e-mail:
>  > dev-unsubscribe@commons.apache.org
>  > For additional commands, e-mail:
>  > dev-help@commons.apache.org
>  >
>  >
>
>
>
>
>
>  ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>  For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message