On Thu, Sep 11, 2008 at 3:55 PM, <[EMAIL PROTECTED]> wrote: > LGTM. I still think you should consider renaming the .defs file to > something else. > > > http://codereview.chromium.org/1935/diff/53/270 > File src/flags.cc (right): > > http://codereview.chromium.org/1935/diff/53/270#newcode50 > Line 50: struct Flag { > I would make this a class since it has methods and everything.
They are the same thing, minus the default public/private. I used a struct to emphasize the lack of constructor, the fact it should be "structure-style" initialized, etc. > > http://codereview.chromium.org/1935/diff/53/270#newcode108 > Line 108: case TYPE_BOOL: > The cases should be indented with two spaces. > > http://codereview.chromium.org/1935/diff/53/270#newcode121 > Line 121: return true; // NOTREACHED > Remove comment and insert an UNREACHABLE() macro call before the return. This was leftover from when I had the code in the header, and couldn't use that macro (since checks.h includes flags.h). Will fix. > > http://codereview.chromium.org/1935/diff/53/270#newcode127 > Line 127: case TYPE_BOOL: > The cases should be indented with two spaces. > > http://codereview.chromium.org/1935 > Ok. Thanks --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
