Page MenuHomePhabricator

Fold `ArcanistPhutilXHPASTLinter` into `ArcanistXHPASTLinter`
ClosedPublic

Authored by joshuaspence on Aug 11 2015, 1:27 PM.
Tags
None
Referenced Files
F14076072: D13867.id34993.diff
Thu, Nov 21, 2:36 PM
Unknown Object (File)
Thu, Nov 21, 1:12 PM
Unknown Object (File)
Thu, Nov 21, 1:06 PM
Unknown Object (File)
Thu, Nov 21, 12:49 PM
Unknown Object (File)
Wed, Nov 20, 1:38 PM
Unknown Object (File)
Wed, Nov 20, 1:38 PM
Unknown Object (File)
Wed, Nov 20, 1:27 PM
Unknown Object (File)
Wed, Nov 20, 1:27 PM
Subscribers

Details

Summary

I've been thinking about this for a while... why not just fold ArcanistPhutilXHPASTLinter into ArcanistXHPASTLinter?

Test Plan

arc unit

Diff Detail

Repository
rARC Arcanist
Branch
master
Lint
Lint Passed
Unit
Test Failures
Build Status
Buildable 7593
Build 8226: [Placeholder Plan] Wait for 30 Seconds
Build 8225: arc lint + arc unit

Event Timeline

joshuaspence retitled this revision from to Fold `ArcanistPhutilXHPASTLinter` into `ArcanistXHPASTLinter`.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.

I don't like that third parties who are developing things like Phabricator applications and arc extensions need to maintain a list of un-disables on these rules. There's no way for them to keep this list in sync with the "correct" list in the upstream.

One possibility that I was considering was to introduce an xhpast.standard which can be phutil or psr-2 (see D13512 for another use case).

Alternatively, we could do something like this:

public function getLintSeverity() {
  if ($this->isPhutilLibrary()) {
      return ArcanistLintSeverity::SEVERITY_ADVICE;
  }

  return ArcanistLintSeverity::SEVERITY_DISABLED;
}

I think I like the "standard" approach. The other one feels maybe a little too magical to me.

epriestley edited edge metadata.

I think this is good to go after D13942.

This revision is now accepted and ready to land.Aug 27 2015, 11:50 AM
This revision was automatically updated to reflect the committed changes.