Prevents linter issues like reported here: https://github.com/Sjors/bitcoin/pull/90#issuecomment-3003870162
But probably also just a good idea.
Also make the use of #!/usr/bin/env bash consistent and added a copyright header (mostly for aesthetic reasons).
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | ryanofsky |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
0@@ -1,3 +1,6 @@
1+#!/usr/bin/env bash
In commit “ci: export LC_ALL” (04eb50505be5700b374d552aaa814422b41ebc33)
I don’t think the config fragments like ci/configs/default.sh should have hashbangs or export LC_ALL because they aren’t marked executable and aren’t designed to be run as standalone scripts. (They only set variables and I wouldn’t want to encourage them to do anything more than that.)
To avoid the lint error, I’d suggest renaming the config files to ci/configs/*.bash instead. This would also be nice because it would indicate that the config files must use bash, not some other variant of shell script. (Note that renaming would also require updating .github/workflows/ci.yml and ci/README.md)
0@@ -1,4 +1,9 @@
1 #!/usr/bin/env bash
2+#
3+# Copyright (c) The Bitcoin Core developers
In commit “ci: add copyright to bash scripts” (fefee4bcdd739b62f9c98df4706b3b5c5ab47087)
Again would prefer to drop copyright from the config fragments, but keep it in the CI scripts. IMO copyright blends pretty well into source code but I find it jarring when it’s injected into things like minor pieces of documentation and config files, and would prefer keep distractions to a minimum in places like that. Ideally these files can just begin with CI_DESC="Description of CI job"
Explicitly opt out of locale dependence.
ci/configs, and renamed them to .bash.
Avoids locale linting error and indicates that the config files
must use bash, not some other variant of shell script.
Code review ACK 3a6db38e561f7645c6e14dd053ee15eb06347d20. Thanks!
I assumed renaming should fix the lint issues based on the *.sh glob pattern in test/lint/lint-shell-locale.py, but will see. Will merge this shortly.