Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T8742: Write a bunch of PSR-2 lint rules
- Commits
- rARCdd0deb240700: Add XHAST linter standards
This still needs work.
Diff Detail
- Repository
- rARC Arcanist
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
This pattern feels a little weird to me:
$standard->configure($this);
In particular, if we extend $standard in the future, calls will probably be passive getter sorts of methods: shouldX(), isYEnabled(), etc.
I'd expect this to be the same way, basically a getDefaultConfigurationValue($key), instead of having the Standard actively reconfigure the Linter. I think that makes the code a little easier to follow usually, too: you can trace the getConfigurationValue(...) call and see that it ends up in the Standard without a chance of missing the earlier active reconfiguration.
Are there reasons not to use that API? The subclasses wouldn't actually change so this is kind of a moot issue, it just felt a bit odd.
Substance of the change seems fine to me. I don't feel strongly about this and we can change it later without breaking the API, so "laziness" is also an acceptable explanation to me.
How do you feel about the name "linter standards"? Another option is " linter ruleset", which is borrowed from PHPCS.
I had also wondered if it makes sense for a linter standard to apply to more than one linter. For example, the phutil linter standard could configure XHPAST and text listing (not a great example because we don't configure the text linter in any way). I decided against it because that seems like the job for the lint engine, hence the supportsLinter method.