[Nix] Add shellcheck to dev shell
ClosedPublic
Actions

Authored by • jon on Aug 8 2022, 9:23 AM.

Details

Reviewers

• abosh
atul
ashoat

Commits

rCOMM16dc802dec6b: [Nix] Add shellcheck to dev shell

Summary

Allow for devs to run shellcheck from nix dev shell

Test Plan

# from any branch
nix develop 'github:commE2E/comm?ref=jonringer/add-shellcheck'
shellcheck --help
which shellcheck # should point to nix store

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• jon created this revision.Aug 8 2022, 9:23 AM

Herald added subscribers: • adrian, tomek. · View Herald TranscriptAug 8 2022, 9:23 AM

Harbormaster returned this revision to the author for changes because remote builds failed.Aug 8 2022, 9:25 AM

Harbormaster failed remote builds in B11211: Diff 15423!

Rebase on latest master

added @ashoat as this is technically "adds" a (although an already approved) dependency.

Harbormaster returned this revision to the author for changes because remote builds failed.Aug 8 2022, 9:35 AM

Harbormaster failed remote builds in B11212: Diff 15424!

• abosh mentioned this in D4772: [lint-staged] Add ShellCheck to `.lintstagedrc.js`.Aug 8 2022, 9:41 AM

iOS build failed because of some intermittent network issue (I think on Buildkite's side w/ Agent API actually), kicked it off again

• abosh added a child revision: D4772: [lint-staged] Add ShellCheck to `.lintstagedrc.js`.Aug 8 2022, 9:56 AM

Harbormaster completed remote builds in B11212: Diff 15424.Aug 8 2022, 9:58 AM

• jon requested review of this revision.Aug 8 2022, 9:58 AM

Looks good to me! Accepting, but adding @ashoat and @atul as blocking as this is adding a new dependency and want at least one of them to take a look at it.

Was able to test this locally and shellcheck worked as expected

This reminded me that our approach to make sure clang-format is accessible to lint-staged is an NPM package... probably a good idea to move that to Nix as well, @jon would you mind creating a task?

This revision is now accepted and ready to land.Aug 8 2022, 3:44 PM

This reminded me that our approach to make sure clang-format is accessible to lint-staged is an NPM package... probably a good idea to move that to Nix as well, @jon would you mind creating a task?

This is already accessible in the nix environment, because stdenv on mac brings in clang, and clang-format is one of the binaries/commands in the clang package