I glanced at the other one. there were a few issues with it: 1. It does more than it says. It changes how the horizontal lines are drawn without explanation (nothing in the changelog and the bug only mentions the other direction). 2. It uses a lot of "magic" numbers and the same ones repeatedly. It would be nice to make constants out of some of them to help inform the reader where they come from. 3. It is unclear if there are any layout tests which cover this change.
I would be happy to r- if you want :) but am uncomfortable with an r+ due to those reasons. On Tue, Oct 13, 2009 at 5:25 PM, Evan Martin <[email protected]> wrote: > > dimich looked at one. Any reviewer for the other? It's like 8 > obvious Linux-specific lines, no stress I promise. :) > > On Tue, Oct 13, 2009 at 4:06 PM, Evan Martin <[email protected]> wrote: > > Both of these patches don't really have an obvious reviewer, but > > they're pretty simple. > > https://bugs.webkit.org/show_bug.cgi?id=30319 > > https://bugs.webkit.org/show_bug.cgi?id=30320 > > > > The latter one will require an epic amount of rebaselining, which I > > have volunteered to do. If anyone has advice on how to do that in a > > way that makes the webkit sheriffs happy, let me know. > > > > > > --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
