Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run shellcheck on nix-installer.sh #995

Merged
merged 3 commits into from
Jun 19, 2024
Merged

Run shellcheck on nix-installer.sh #995

merged 3 commits into from
Jun 19, 2024

Conversation

lucperkins
Copy link
Member

@lucperkins lucperkins commented Jun 7, 2024

Description

At the moment, we do have a shellcheck directive in nix-installer.sh but we don't actually run shellcheck against it. Doing so has exposed two very minor infractions but worth automating this.

Checklist
  • Formatted with cargo fmt
  • Built with nix build
  • Ran flake checks with nix flake check
  • Added or updated relevant tests (leave unchecked if not applicable)
  • Added or updated relevant documentation (leave unchecked if not applicable)
  • Linked to related issues (leave unchecked if not applicable)
Validating with install.determinate.systems

If a maintainer has added the upload to s3 label to this PR, it will become available for installation via install.determinate.systems:

curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix/pr/$PR_NUMBER | sh -s -- install

@lucperkins lucperkins changed the title Run shellcheck on nix-installer.sh Run shellcheck on nix-installer.sh Jun 7, 2024
@cole-h
Copy link
Member

cole-h commented Jun 7, 2024

This script is lightly edited from the rustup init script (https://github.com/rust-lang/rustup/blob/master/rustup-init.sh). Maybe you could also send this diff upstream to them? Their script is fairly robust, so it would be sad to drift terribly far from what they're doing just to satisfy shellcheck. (I know this change isn't that much of a diff, but if shellcheck ever becomes unhappy about larger parts...)

@lucperkins
Copy link
Member Author

@cole-h Okay, I've determined that we actually do not want to quote $_retry here so I've added disable statements instead.

@grahamc grahamc merged commit 1998fe1 into main Jun 19, 2024
16 checks passed
@grahamc grahamc deleted the shellcheck-script branch June 19, 2024 14:39
@cole-h cole-h added this to the 0.20.0 milestone Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants