-
Notifications
You must be signed in to change notification settings - Fork 1
Description
Great work and thanks for sharing the mac setup scripts.
You asked for feedback on Reddit. Here are a few ideas that you may want to consider; these are based on a lot of longtime lessons learned with my own mac setup scripts.
Shell
Prefer zsh over bash, because macOS now defaults to zsh.
Color naming
For color name constants, prefix with "COLOR_" like this;
- RED='\033[0;31m'
+ COLOR_RED='\033[0;31m'And especially for color reset:
- NC='\033[0m' # No Color
+ COLOR_RESET='\033[0m'This convention makes it much easier to search code for all colors.
Log naming
For log function names, aim for Unix conventions plus trace, using these naming conventions:
- LOG_EMERG=0 (Emergency: System is unusable)
- LOG_ALERT=1 (Alert: Action must be taken immediately)
- LOG_CRIT=2 (Critical conditions)
- LOG_ERR=3 (Error conditions)
- LOG_WARN=4 (Warning conditions)
- LOG_NOTICE=5 (Notice: normal but significant conditions)
- LOG_INFO=6 (Informational)
- LOG_DEBUG=7 (Debug-level messages)
- LOG_TRACE=8 (Trace-level messages)
This convention makes it much easier to filter log messages by level, such as by using syslog conventions.
This also means slightly-renaming your log functions, such as:
- log_error
+ log_errExit codes
If you want to be friendly to developers, specify your exit code constants up front, and use Unix conventions, such as the exit code constant for an operating system error:
EXIT_OSERR=71Create a die function
You'll likely use this a lot in many scripts, because it's so common to log a message about the system being unusuable, then exit with an exit code.
die() {
n="$1" ; shift ; log_emerg "$*"; exit "$n"
}Function versus comment and bare code
Prefer creating a function with an exit code constant, rather than creating a comment and bare code with an exit code number.
You can see the pattern in your own code:
# Check if running on macOS
if [[ "$OSTYPE" != "darwin"* ]]; then
log_error "This script is designed for macOS only"
exit 1
fiRefactoring:
is_macos() {
[ "$OSTYPE" == "darwin"* ]
}
require_macos() {
is_macos || die $EXIT_OSERR "This script is designed for macOS only"
}Use case
Prefer a case statement instead of if..elif..etc when you're matching on the same variable.
Existing code:
if [[ "$ARCH" == "arm64" ]]; then
ARCH_NAME="Apple Silicon"
BREW_PREFIX="/opt/homebrew"
elif [[ "$ARCH" == "x86_64" ]]; then
ARCH_NAME="Intel"
BREW_PREFIX="/usr/local"
else
log_error "Unknown architecture: $ARCH"
exit 1
fi
Refactoring makes your code more direct, and simpler to understand, and easier to extend:
case "$ARCH" in
arm64)
ARCH_NAME="Apple Silicon"
BREW_PREFIX="/opt/homebrew"
;;
x86_64)
ARCH_NAME="Intel"
BREW_PREFIX="/usr/local"
;;
*)
die $EXIT_OSERR "Unknown architecture: $ARCH"
;;
esacAll of the above ideas are for you to consider as refactoring ideas, that will IMHO help your coding career grow, and also will favor good coding techniques such as testing, or tranforming to python or rust, or using AI to improve the code over time.
Thanks again for creating your script and sharing it <3