review request for 7008713: diamond conversion of kerberos5 and security tools

[Stuart Marks]

On 12/22/10 5:45 PM, Weijun Wang wrote:
> Looks fine.

Great!

> 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 
works out.

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 
to Sean.

> 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.

s'marks