Page MenuHomePhabricator

[DOC] Improve instructions for setting up shellcheck
ClosedPublic

Authored by Fabien on Jul 29 2020, 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.

Event Timeline

Fabien requested review of this revision.Jul 29 2020, 13:31
jasonbcox requested changes to this revision.Jul 29 2020, 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.Jul 29 2020, 16:28
Fabien planned changes to this revision.Jul 29 2020, 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.

Make it appear as a note and deduplicate version check

This revision is now accepted and ready to land.Jul 29 2020, 22:24