review request for 7008713: diamond conversion of kerberos5 and security tools
stuart.marks at oracle.com
Wed Dec 22 18:04:43 PST 2010
On 12/22/10 5:45 PM, Weijun Wang wrote:
> Looks fine.
> BTW, what are we supposed to review? Besides going through the patch and making
> sure each change is good, the only thing I can think of is looking for lines
> need coinification but untouched. Made some simple greps and found none yet.
A fair question. While most of the changes are mechanical, I think the main
criterion is, "does this improve the code?" The diamond operator doesn't (or at
least, shouldn't) change any semantics of the code, so this is all about style,
readability, conciseness, etc.
Using the diamond operator definitely makes the code *shorter*. Whether it's
*better* doesn't necessarily follow from that. In most cases, though, using
diamond is better, I think.
I'd be curious if you saw any instances where you thought that it would be
better not to use diamond. It's not a requirement to use diamond everywhere
possible. That's one of the points of this exercise, which is to put the new
language features through their paces on real code, and to see how well it
The Jackpot diamond finder is pretty darned good at finding candidates to
convert. It's certainly a lot better than I could do. However, I did find one
instance that I think it missed; see the review request for 7008728 I just sent
> I can review the following files you listed for Vinnie: [...]
OK, thanks, I'll prepare another webrev for this chunk, either tomorrow or
sometime next week.
More information about the security-dev