If there are consistent patterns of NOLINT usage, I can suppress the whole error class.
Also, the linter is only automatically run on chrome/ and app/, IIRC. -- Elliot On Wed, Dec 9, 2009 at 4:38 PM, Brett Wilson <[email protected]> wrote: > 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 > -- Chromium Developers mailing list: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
