code review request for 6880112, Coin: use diamond in core libraries
Joe Darcy
joe.darcy at oracle.com
Fri Dec 17 03:18:39 UTC 2010
On 12/16/2010 7:14 PM, Stuart Marks wrote:
> Thanks for the comments, folks.
>
> I've been puzzling over the issue of joining lines after diamond
> conversion if they're short enough. It turns out to be a surprisingly
> complex interaction of tools, code style, and process.[1]
>
> Joe and I actually talked about this issue briefly the other day and
> our initial inclination was not to make the additional whitespace
> changes. In some sense the Jackpot automated converter should do this
> for us, but it doesn't, and I don't want to take the time at the
> moment to get up to speed and modify it to do this.
>
> However, Brian and Alan and another person off-list have all raised
> this issue, so I think it's important that we address it. Looking at
> it again, leaving something like this in the code:
>
> private static Map<String, URL> urls
> = new HashMap<>(10);
>
> really offends my sensibilities, and let's face it, nobody is going to
> go in and fix this unless I do it now. It turns out that it isn't too
> difficult to write a script that can be run after-the-fact to join
> these lines if they end up shorter than 80 characters.
>
> Unfortunately, as Kelly points out (and I've also verified this)
> changing the line breaks will change the .class file contents (though
> not the bytecodes). The original changeset does have a very nice
> property of not changing the .class files at all. We could preserve
> this property by putting the line joins in a separate changeset. I'd
> do this if we could have multiple changesets with the same bugid, but
> a separate changeset requires a separate bugid... so maybe this isn't
> worth the overhead after all.
>
> I've run the line joiner and it changed ten files. Here's an
> incremental webrev, over and above yesterday's:
>
> http://cr.openjdk.java.net/~smarks/reviews/6880112/webrev.1/
>
> and the corresponding hg bundle is here:
>
> http://cr.openjdk.java.net/~smarks/reviews/6880112/bundle.1.hg
>
> If this doesn't meet with any objections, I'll merge these whitespace
> changes and the earlier diamond conversion changes into a single
> changeset and then push the result.
>
Looks good to me - approved.
-Joe
More information about the core-libs-dev
mailing list