On Wed, Dec 9, 2009 at 4:24 PM, John Abd-El-Malek <j...@chromium.org> wrote:
> btw I searched the code, almost all the instances are in code from different
> repositories, like v8, gtest, gmock. I counted only 17 instances in
> Chrome's code.
Most of the Chrome NOLINTs look like the're around ifdefs, where the
ifdef code sometimes mean that a comma or a semicolon goes on the line
after the ifdef. We should be working to remove these anyway since the
ifdefs are super ugly, and I'm not sure the NOLINT comment actually
makes them worse. Some of these may not be practical or desirable to
remove, though.
So I don't think I see a big problem with the way NOLINT is used in
Chrome. Looking through V8 I don't see a huge problem either.
Some NOLINT calls weren't clear to me why the linter complained. I
suggest that NOLINT comments be documented. In some places they
already are. Then reviewers will know to argue when they see something
untoward, whereas just "// NOLINT" isn't alwasy clear about what the
problem is and whether they should complain. This will also make
NOLINTs more painful to add since you have to justify why you're
adding it, which will hopefully decrease its usage.
Brett
--
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
