commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: [VOTE] Release BCEL 6.0 based on RC3
Date Fri, 06 Mar 2015 23:19:06 GMT
On 06/03/2015 21:05, Sarah Murray wrote:
>    - 187 <https://issues.apache.org/jira/browse/BCEL-187> - has a test
>    case. no committer review for 2 months

Neither has anyone else reviewed it. It isn't just committers that can
review patches. I've got the necessary commit karma and after looking at
that issue for a minute or two I can see a whole pile of work that needs
to be done before I'd even think about committing anything. Just from
that quick look:
1. Review the svn history to see if there are any pointers to which part
of the JVM spec defines the restriction.
2. Review the JVM spec to see if I can see the restriction
3. Look at the byte code for the provided example to see which class the
invoke opcode references. Better yet, write a disabled BCEL unit test to
show this.
4. The structure of the test case looks odd. I suspect it aligns with
other patches for other issues but nowhere is that explained.
5. The class files provided do not have any attached source. Without the
source code, we have no idea (OK I could fire up a decompiler but that
is another task) what is actually in those classes.

To put it another way, the current code asserts one behaviour. The bug
report asserts another. No evidence is provided to support either
position. Given that most (all?) the current committers likely to work
on BCEL don't have a deep knowledge of the JVM specs some research is
required to figure out which is correct. I've set out above how I'd do
the research. I'm sure there are other, better approaches. Doing the
research doesn't require commit karma.

>    - 188 <https://issues.apache.org/jira/browse/BCEL-188> - has a test
>    case. no committer review for 2 months

Looks to be in better shape than 187 but still no source provided for
one of the test classes.

>    - 183 <https://issues.apache.org/jira/browse/BCEL-183> - has a test
>    case. no committer review for 3 months

It is not at all clear how much of the original issue (if any) was fixed
by the change reported 19-Dec-2014.

The potential for the required behaviour to vary with class file version
is a complicating factor.

This issues seems to be bundling up a number of smaller issues. That
makes it harder to work with even if they are all in the same area.

Generally for all of these issues some pointers to the relevant parts of
the JVM spec - preferably as comments in the patch - would make it
easier for folks to review the patches and easier for future maintainers
to understand what the code is doing.

Also generally, it looks like a number of the patches could be reduced
in size by pulling out the common testing framework.


> The problem is that there's one area where help is really lacking and the
> community can't contribute there because not everyone has the commit access
> that's needed to get things flowing again.

I disagree with that analysis. There is plenty that could be done to
move these issues forwards without commit access. I can find the odd
hour here and there to look at BCEL issues but looking at the three
issues you quoted above all of them would require far more than an hours
work to get to a position where I'd be happy to commit something. Pretty
much all of that work could be done by anyone on this list.

> Perhaps we could add more committers since it's currently a bottleneck?

Committers will be added as and when folks demonstrate the appropriate
merit.

> E.g. Mark and Jerome both have submitted numerous high quality patches and
> seem to be well-regarded members of the community. Can we give them commit
> access?

I agree they look to be heading in the right direction. I'm not familiar
enough with the BCEL code base to judge the quality of the patches in
their current form.

> If we're nervous about maintaining high code quality we can add
> guidelines such as there must be a test case for any commit and you can't
> commit your own code - someone else needs to review it.

You really don't want to introduce those sorts of rules if you can help
it. It would slow progress even further.

> There was talk of an alpha release and it seems like folks are in favor of
> it. Is there anything I can help to do with regards to that?

Right now the most useful thing folks can do is pick a bug that matters
to them and then work with the community to get the issue to the point
where a current committer can resolve it with no more than 60 minutes of
effort.

I can find 60 minutes over the next few days. Which issue do you want to
fix first?

I'm also more than happy to commit common test code to reduce the
size/complexity of any proposed patches.

Mark

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


Mime
View raw message