Page MenuHomePhabricator

Implement a very rough version of saving lint from Harbormaster
AbandonedPublic

Authored by hach-que on May 14 2014, 3:35 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 19, 3:36 AM
Unknown Object (File)
Fri, Apr 19, 3:36 AM
Unknown Object (File)
Fri, Apr 19, 3:36 AM
Unknown Object (File)
Fri, Apr 19, 3:36 AM
Unknown Object (File)
Fri, Apr 19, 3:36 AM
Unknown Object (File)
Fri, Apr 19, 3:36 AM
Unknown Object (File)
Thu, Apr 11, 4:48 AM
Unknown Object (File)
Wed, Apr 3, 9:21 AM

Details

Summary

This is a very rough version of running arc lint on a Drydock agent and saving the information into Diffusion.

There's a few hacks for Windows here; in particular using %C instead of %s because cmd.exe doesn't like single quotes. It'd be good to have a mode for csprintf to tell it to do Windows-style escaping even through the current machine is UNIX.

Another issue is the race condition-ness of this implementation; technically it could pass the "is there a branch pointing to this", start linting for one commit and then another later commit also triggers and starts linting. This is mostly mitigated by the fact that unless "Lint Everything" is turned on the linting operations will be incremental, but entirely mitigated if "Wait for Previous Builds" is placed before the lint step in a build plan.

Finally, this won't populate the authorPHID field in the lint messages; i.e. it doesn't calculate blame information for the files that are linted. I would like to address this with T5001 at some point so we don't need to recalculate blame every time, to reduce the need for waiting, and to introduce historical aggregated information.

Test Plan

Ran it against an internal codebase, all of the wonderful lint messages turned up.

Diff Detail

Repository
rP Phabricator
Branch
arcpatch-D9113
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 2072
Build 2073: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

hach-que retitled this revision from to Implement a very rough version of saving lint from Harbormaster.
hach-que updated this object.
hach-que edited the test plan for this revision. (Show Details)
hach-que added a reviewer: epriestley.
hach-que edited edge metadata.

Fixing lint errors

Allow variables in project path and add ugly hack on Windows

Also, this implementation only matches the first branch that's resolved; i.e. if two branches point to the commit, only the first one will actually get the lint data associated with it.

Add command to change drive letter for Windows

src/applications/harbormaster/step/HarbormasterLintSaveBuildStepImplementation.php
192–220

right here

epriestley edited edge metadata.

This should wait for T8095 and be rebuilt on top of that.

This revision now requires changes to proceed.May 13 2015, 2:00 PM

Beyond the initial implementation of this, we haven't seen much use of it (nor have we seen much usage of the statistics it generated).