On 7/10/19 9:54 AM, Peter Levart wrote:
On 7/10/19 6:03 PM, Joe Darcy wrote:
Hello,
I'd advise against including at least part of the comment
// declare serialization compatibility with JDK 8 (see JDK-8227368)
We generally don't include bug numbers in the text of the code and rely
mapping from the SCM to the bug database to provide the (sometimes large!)
additional context for a fix.
Ok, Joe. I removed the reference to bug number and recreated webrev.03 in-place.
Will that be OK?
http://cr.openjdk.java.net/~plevart/jdk-dev/8227368_EnumSet.class_serialization/webrev.03/
Hi Peter,
I'm ok with this patch (i.e., without the bug id). I did a test run and it looks
good. Given that Joe approved the CSR, you're now cleared to push. Remember to
push to JDK 13!
We should probably discuss the JDK 11 backport on jdk-updates, since it'll
differ from the JDK 13 patch.
Joe,
For this patch having the bug id doesn't matter very much, since there is
nothing unusual about this code as it stands, and it'll be ok for the long term.
However, there are a bunch of places in the source code where we do reference
bug reports; they're pretty easy to find. There are perhaps over 100 in
java.base alone. (Some include full URLs, which I think is unnecessary.)
I think it's easier to go directly to a bug report than to search through the
history for bug ids in changeset comments. "hg anno" works reasonably well
initially, but as time goes on the changeset associated with a particular line
of code can be obscured by incidental refactorings, file renames, etc.
For example, I expect the JDK 11 backport to assign the serialVersionUID in a
static initializer block (which will require warnings suppression as well). This
is unusual and warrants a comment. This could either be a complete explanation
(which might run to a paragraph or more) or a reference to a bug id. Sometimes
an in-line explanation is warranted, but there's enough history here that in
this case a reference to a bug id seems more appropriate.
s'marks
