-
Notifications
You must be signed in to change notification settings - Fork 208
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
WIP: Tidy up preload script #2595
Conversation
the if/else conversion to case statements are fine but I fail to see how anything else is an improvement... it looks harder to read/understand and is multiple lines longer. |
also shellcheck shows a syntax error. refer to the action log |
@@ -37,25 +37,18 @@ function error { | |||
exit 1 | |||
} | |||
|
|||
#yad or xlunch format | |||
case "$1" in | |||
source) return 0 ;; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The preload
script is not to be sourced. Why has this input argument been added?
fi | ||
# The prefix is a category to preload. By default (if '/' or empty) the main | ||
# page is preloaded. | ||
test "$2" = / || prefix="$2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this breaks the prefix variable and thus the preload script, it is no longer set when it is not /
Closing as stale as the author has not implemented requested changes and the if statement to case conversion makes no functional difference (we can implement that whenever if we wanted). |
This is a set of optimizations and cleanups for preload.