scripts: split setup-stack.sh#3115
Conversation
NO CHANGE TO FUNCTIONALITY, JUST COPYING
The variables script is actually kinda like a stack for us; what you additionally need to know is that the declaration of the arrays is weirdly scoped in Bash; hence #3112 takes such an unusual approach at placing the declarations. With variables being a stack, everything becomes a bit nicer.
georglauterbach
added
area/scripts
kind/improvement
polarathene
left a comment
polarathene
left a comment
There was a problem hiding this comment.
Manually compared diffs before/after, LGTM 😅 👍
Cheers for handling this step as a separate PR 🎉
| source "${PATH_TO_SCRIPTS}/accounts.sh" | ||
| source "${PATH_TO_SCRIPTS}/aliases.sh" | ||
| source "${PATH_TO_SCRIPTS}/change-detection.sh" | ||
| source "${PATH_TO_SCRIPTS}/dhparams.sh" |
There was a problem hiding this comment.
This files contents could probably live in the ssl.sh helper as it's related to that support (ciphers)
There was a problem hiding this comment.
I will merge this PR (to get it through), and will apply this feedback in the next, smaller PR :)
| do | ||
| # shellcheck source=/dev/null | ||
| source "${FILE}" | ||
| done < <(find /usr/local/bin/setup.d/ -type f) |
There was a problem hiding this comment.
Generally bad practice - files including spaces for example will not be handled correctly. You should add -print0 to the find command and define a delimiter -d $'\0' in the read command or just use globbing instead:
for i in /usr/local/bin/setup.d/*.sh; do source "$i"; doneThis should also be faster, because no external program needs to be run.
There was a problem hiding this comment.
This will require shopt -s globstar since we need to use /usr/local/bin/setup.d/**/*.sh, but I'm absolutely fine with that! I will provide a PR.
There was a problem hiding this comment.
You are right, I overlooked that there is also a subdirectory and not just files.
3 participants
Description
Superseeds parts of #3112. Makes review easier. Split
setup-stack.sh, did not add / remove any functionality. Basically a glorified copy-paste with minor renamings.Fixes issue where reviewing #3112 is a chore, I guess.
I will leave #3112 open so I can gradually implement all the changes this single PR would bring. Having reviewed #3112 is good because you basically know what's coming @polarathene, so no time wasted here.
The first commit is basically just copy-paste everywhere, the rest is more renaming/minor adjustments.
This PR is the first part of a series to come about refatoring some parts of the setup with the final goal of disallowing faulty restarts.
This PR is BIG, but actually 95% copy-paste; there are no changes in functionality whatsoever.
Type of change
Checklist:
docs/)