Skip to content

Fix shellcheck issues in two scripts#946

Merged
mhucka merged 3 commits intoquantumlib:mainfrom
mhucka:mh-fix-shell-scripts
Nov 8, 2025
Merged

Fix shellcheck issues in two scripts#946
mhucka merged 3 commits intoquantumlib:mainfrom
mhucka:mh-fix-shell-scripts

Conversation

@mhucka
Copy link
Copy Markdown
Collaborator

@mhucka mhucka commented Nov 7, 2025

This fixes:

  • hash-bang line was not the first line in the file
  • some variables needed to be quoted

Reported by shellcheck.

Changes:

- A past run of `addlicense` must have misshandled the hash-bang line in
these shell scripts.

- Some variables needed to be wrapped in double quotes.
@mhucka mhucka requested review from fdmalone and pavoljuhas November 7, 2025 22:03
@github-actions github-actions Bot added the size: S 10< lines changed <50 label Nov 7, 2025
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly moves the shebang to the first line in both scripts, fixing a shellcheck issue. However, the changes to quote the CUSTATEVECFLAGS variable are incorrect and will break the build. Quoting a variable that contains multiple command-line options causes the shell to pass them as a single argument, rather than separate ones. My review provides critical feedback on these lines, explaining why the change is wrong and offering two solutions: the most robust one using bash arrays, and a more minimal one that reverts the incorrect change while correctly quoting other path variables. These changes are critical to ensure the scripts remain functional.

Comment thread apps/make.sh Outdated
Comment thread tests/make.sh Outdated
@github-actions github-actions Bot removed the size: S 10< lines changed <50 label Nov 7, 2025
@github-actions github-actions Bot added the size: M 50< lines changed <250 label Nov 7, 2025
@mhucka mhucka added this pull request to the merge queue Nov 8, 2025
Merged via the queue into quantumlib:main with commit 3b2140d Nov 8, 2025
52 checks passed
@mhucka mhucka deleted the mh-fix-shell-scripts branch November 8, 2025 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: M 50< lines changed <250

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants