On Wed, Dec 9, 2009 at 4:24 PM, John Abd-El-Malek <[email protected]> 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: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
