Page MenuHomePhabricator

Minor fixes to arcanist cpplint
ClosedPublic

Authored by michaeloa on Jan 14 2016, 2:53 PM.
Tags
None
Referenced Files
F14077298: D15019.diff
Thu, Nov 21, 10:32 PM
Unknown Object (File)
Tue, Nov 19, 6:12 PM
Unknown Object (File)
Sat, Nov 16, 7:48 AM
Unknown Object (File)
Mon, Nov 11, 5:22 PM
Unknown Object (File)
Sat, Nov 9, 11:12 AM
Unknown Object (File)
Fri, Nov 8, 7:05 PM
Unknown Object (File)
Fri, Nov 8, 5:04 AM
Unknown Object (File)
Mon, Nov 4, 5:09 PM
Subscribers

Details

Summary

This fixes the regex in "getLintCode..." so that it accepts lint
codes such as build/c++11 which have become valid lint codes
in later versions of cpplint.

It also corrects the install instructions for the linter (Google's
style guide is no longer available on SVN and has been migrated to
Github).

Test Plan

For the Regex:

  • Create an .arclint in a project such as:
{
  "linters": {
    "cpplint": {
      "type": "cpplint",
      "severity": {
        "build/c++11": "advice"
      }
    }
  }
}
  • Run arc lint with the existing linter. This should fail. Patch the linter, and this should now be accepted.

For the Instructions

  • Verify the download location wget https://raw.github.com/google/styleguide/gh-pages/cpplint/cpplint.py

Diff Detail

Repository
rARC Arcanist
Branch
T10118 (branched from master)
Lint
Lint Passed
Unit
Tests Skipped
Build Status
Buildable 10106
Build 12249: arc lint + arc unit

Event Timeline

michaeloa retitled this revision from to Minor fixes to arcanist cpplint.
michaeloa updated this object.
michaeloa edited the test plan for this revision. (Show Details)
michaeloa added a reviewer: epriestley.

Not sure how to test the .arclint syntax with the existing test framework, and didn't find any good examples of doing this among the other linters that I browsed. If you have any tips or good examples to look at, I'd be happy to add some additional tests.

The issue is that parsing fails, right? It's sufficient to just make sure parsing succeeds without actually verifying that the severity map works.

So I'd expect the test case to look something like this:

something_that_generates_a_build/c++11() messsage
~~~~~
advice:1

Before your patch, that should fail; after your patch it should pass.

epriestley edited edge metadata.

Oh, nevermind. I thought it was the other regex. This is fine as is, there's no convenient way to test it right now.

src/lint/linter/ArcanistCpplintLinter.php
39

Oh! I thought this was the regexp that was wrong.

This revision is now accepted and ready to land.Jan 14 2016, 2:58 PM
This revision was automatically updated to reflect the committed changes.