Page MenuHomePhabricator

[LINTER] Integrate lint-locale-dependence.sh into arcanist
ClosedPublic

Authored by Fabien on Mar 19 2019, 09:12.

Details

Summary

Change the linter to accept a file as an argument, and always return 0
to avoid arcanist to throw a script execution error.
Add a php class to interpret the linter output and display linting error
messages through arc lint.

Depends on D2704

Test Plan
arc lint

Comment any of the known exception in the linter-locale-dependence.sh
file and add a comment in the corresponding source file. Check that `arc
lint` displays errors for the source file.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
linter_locale_arcanist
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 5288
Build 8638: Bitcoin ABC Buildbot (legacy)
Build 8637: arc lint + arc unit

Event Timeline

jasonbcox requested changes to this revision.Mar 22 2019, 22:02
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
arcanist/linter/LocalDependenceLinter.php
57 ↗(On Diff #7743)

Require -> Requires

73 ↗(On Diff #7743)

<funtion> -> <function>

80 ↗(On Diff #7743)

theses -> these

test/lint/lint-locale-dependence.sh
227 ↗(On Diff #7743)

The php script indicates that the linter throws errors, but why is it that the script always returns success?

This revision now requires changes to proceed.Mar 22 2019, 22:02
test/lint/lint-locale-dependence.sh
227 ↗(On Diff #7743)

This is due to the way arcanist handle the error codes.
If the script returns non zero, arcanist consider that it failed to run and so the linter output should not be parsed.
Errors in the php file refer to the linting errors which are printed on stdout and then parrsed by the php script.
"Error" might not be the best word here, any suggestion ?

jasonbcox added inline comments.
test/lint/lint-locale-dependence.sh
227 ↗(On Diff #7743)

I think just differentiating between "linting errors" and "script errors" would be helpful enough. Thanks for the explanation.

This revision is now accepted and ready to land.Mar 25 2019, 17:55

Rebase and improve comments/summary to clarify script errors vs linting errors

This revision was automatically updated to reflect the committed changes.