Skip to content

fix: make shell script changes recommended by ShellCheck#948

Merged
RoyalOughtness merged 18 commits intosecureblue:livefrom
HastD:shellcheck
Mar 20, 2025
Merged

fix: make shell script changes recommended by ShellCheck#948
RoyalOughtness merged 18 commits intosecureblue:livefrom
HastD:shellcheck

Conversation

@HastD
Copy link
Copy Markdown
Collaborator

@HastD HastD commented Mar 18, 2025

This PR contains various minor changes to shell scripts suggested by ShellCheck. None of these changes should substantively change script behavior except to fix outright errors. The only major errors I found are in the part of audit.just that checks for sysctl overrides (which was completely broken and now works, at least on my system)—reviewers, please double-check my work on that.

@RoyalOughtness
Copy link
Copy Markdown
Collaborator

looks like there are a couple actions we can use too? https://github.com/marketplace?query=ShellCheck&type=actions

@HastD HastD marked this pull request as ready for review March 18, 2025 21:57
@HastD HastD requested a review from RoyalOughtness as a code owner March 18, 2025 21:57
@HastD
Copy link
Copy Markdown
Collaborator Author

HastD commented Mar 18, 2025

Okay, this is ready for review. I don't know why the automated tests are failing, btw—it's saying the justfile variable {{ BOLD }} isn't defined, but the documentation lists it as a predefined constant, and it does work as expected when I run the actual scripts.

RoyalOughtness
RoyalOughtness previously approved these changes Mar 18, 2025
RoyalOughtness
RoyalOughtness previously approved these changes Mar 19, 2025
@RoyalOughtness RoyalOughtness merged commit 6b3bc1b into secureblue:live Mar 20, 2025
8 checks passed
@HastD HastD deleted the shellcheck branch March 20, 2025 14:25
RoyalOughtness pushed a commit to RoyalOughtness/secureblue-dev that referenced this pull request Aug 4, 2025
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.

4 participants