Mike,
Here is the new revision:
http://cr.openjdk.java.net/~ksrini/6990106/webrev.03/
I nailed all the items per your suggestions.
A few more I nits I noticed.
BandStructure : allKQBands could be final.
ClassReader : string switch opportunities in readAttributes()
FixedArrayList : Seems to
Hi Mike,
Thanks for the review, here is the new version:
http://cr.openjdk.java.net/~ksrini/6990106/webrev.02/
Attribute.java/Instruction.java/Package.java.File :
- Layout.equals(Object x) {
return x instanceof Layout equals((Layout)x);
}
should be :
Layout.equals(Object x) {
return
A few more I nits I noticed.
BandStructure : allKQBands could be final.
ClassReader : string switch opportunities in readAttributes()
FixedArrayList : Seems to be unchanged since last webrev. Was nothing saved by
extending AbstractList?
ConstantPool : equals() only works as long as all
Attribute.java/Instruction.java/Package.java.File :
- Layout.equals(Object x) {
return x instanceof Layout equals((Layout)x);
}
should be :
Layout.equals(Object x) {
return (null != x) (x.getClass() == Layout.class) equals((Layout)x);
}
as sub-classes also using instanceof would have
Hi John, et. al.,
This revision contains all fixes noted by Brian below and other
comments from Alan.
http://cr.openjdk.java.net/~ksrini/6990106/webrev.01/
Thanks
Kumar
Mostly style issues, but at least one likely bug.
In a few places, you've used @SuppressWarnings(unchecked). While
this
Hi,
Appreciate a review of the pack200 cleanup work, in the following areas:
1. General generification of Collections.
2. fixed worthy findbugs items.
3. fixed worthy Netbeans nags.
4. Elimination of array/generics combination.
The webrev is here:
Mostly style issues, but at least one likely bug.
In a few places, you've used @SuppressWarnings(unchecked). While this may
well be unavoidable, it is worth factoring this out into the smallest possible
chunk. For BandStructure, you've applied it to the whole class, and there's
some big
Hi Alan, Brian,
Thanks! I will look into both your comments.
Kumar - I just scanned this (sorry, I don't have time to do a detailed
review).
- is Attribute.isEmpty right? or rather should ==null be replaced by
false?
yes, I will redo this.
- in BandStructure.java, can basicCodingIndexes
So some of these changes are using the new jdk7 language features?
(diamond operator)
Is this a first, or is this happening already? Just curious.
decided to use it, put NB on autopilot. :-)
Le 11/11/2010 20:34, Kelly O'Hair a écrit :
So some of these changes are using the new jdk7 language features?
(diamond operator)
Is this a first, or is this happening already? Just curious.
It's not the first time. I have seen it time to time.
Mark (Reinhold) was the first to push a patch
10 matches
Mail list logo
