Page MenuHomePhabricator

[DOC] Improve instructions for setting up shellcheck
ClosedPublic

Authored by Fabien on Wed, Jul 29, 13:31.

Details

Reviewers
jasonbcox
Group Reviewers
Restricted Project
Commits
rABCa02d9fce3d58: [DOC] Improve instructions for setting up shellcheck
Summary

This diff updates the CONTRIBUTING.md to give more info on how to
setup shellcheck from the precompiled binaries, and check the correct
version is in the PATH.

Depends on D7074.

Test Plan

Read the doc and try the instructions.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Fabien created this revision.Wed, Jul 29, 13:31
Herald added a reviewer: Restricted Project. · View Herald TranscriptWed, Jul 29, 13:31
Fabien requested review of this revision.Wed, Jul 29, 13:31
jasonbcox requested changes to this revision.Wed, Jul 29, 16:28
jasonbcox added a subscriber: jasonbcox.

This doesn't strike me as something that belongs in CONTRIBUTING and brings up lots of questions:

  • Why do we expect new devs to have multiple shellcheck versions installed?
  • Why do we expect shellcheck to not be in PATH correctly?
  • What makes shellcheck install special such that we need to tell devs to check the version, but we do not do this for other tools?
This revision now requires changes to proceed.Wed, Jul 29, 16:28
Fabien planned changes to this revision.Wed, Jul 29, 20:21

This is expected to give a hint on why arcanist could fail to use a recent shellcheck version freshly downloaded.
Talked offline, this will be simplified a bit to not require more steps for the user.

Fabien updated this revision to Diff 22618.Wed, Jul 29, 20:25

Make it appear as a note and deduplicate version check

jasonbcox accepted this revision.Wed, Jul 29, 22:24
This revision is now accepted and ready to land.Wed, Jul 29, 22:24
This revision was automatically updated to reflect the committed changes.