OK, thanks for fixing up this stuff, even before I had mentioned it.

As far as I'm concerned the change is ready to go in, but as you mention we still need to get (internal) approval for spec changes.

s'marks

On 3/4/14 1:12 PM, Brian Burkhalter wrote:
Hi Stuart,

On Mar 4, 2014, at 1:09 PM, Stuart Marks wrote:

Just a couple small items.

At line 4203, the type of magnitude should be byte[] instead of int[]. Whoops,
I could have sworn I wrote that in my previous review, but it must have gotten
dropped while I was editing. Sorry about that.

I caught that myself and fixed it.

For the four compatibility fields in the serial form, the comment is

   appears in the serialized for backward compatibility

Something is missing here. Should it say "appears in the serialized form for
backward compatibility" ?

I also caught and fixed that.

The comment block at lines 4300-4306 is good. I might also add a note to say
these values are compatible with older implementations.

Will do (I'll refresh the updated webrev at the link below).

There are some things in the serialization doc that ought to be brought up
to date, though. Note that the docs for serialPersistentFields, readObject,
and writeObject appear in the javadoc output, in the "Serialized Form" page,
even though these members are private!

Isn't this controlled by options passed to the javadoc tool as opposed to
settings in the source code?

No, serialization is "special" in that all information about the serialized
form, including the docs these special private methods and fields, do appear
in the Serialized Form output, regardless of the javadoc tool arguments.

Thanks for the clarification.

I think I'll need another "thumbs up" as this has changed since Paul's
approval was posted.

Paul is not available this week. If you want to make these corrections and
then just push the changeset, it's fine by me; I think it's had enough review.

Sounds good.

http://cr.openjdk.java.net/~bpb/8035279/webrev.03/

I think I have to get a CCC request approved first however but if the approval
is in place I can push immediately thereafter.

Thanks,

Brian

Reply via email to