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
