fix(review): address PR #2 inline review findings
- commit
- dd03646b63eb079aa994e7f88638911cf278d83f
- author
- brice · · 2026-02-27 04:57 UTC
- parent
- 74791d1
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
7 files · +15 -6
M install/mandatory/post-boot/cli-tools.sh +3 -3
| @@ -9,14 +9,14 @@ USER_HOME=$(getent passwd "$USERNAME" | cut -d: -f6) | |||
| 9 | 9 | log() { echo "[cli-tools] $*" | tee -a "$LOG_FILE"; } | |
| 10 | 10 | ||
| 11 | 11 | if [[ -z "$USERNAME" ]]; then | |
| 12 | - | log "ERROR: Could not detect user in /home" | |
| 12 | + | log "ERROR: Could not detect user via getent passwd" | |
| 13 | 13 | exit 1 | |
| 14 | 14 | fi | |
| 15 | 15 | ||
| 16 | 16 | log "Installing modern CLI toolkit for $USERNAME..." | |
| 17 | 17 | ||
| 18 | - | log "Refreshing package databases..." | |
| 19 | - | pacman -Sy --noconfirm | |
| 18 | + | log "Upgrading system and refreshing package databases..." | |
| 19 | + | pacman -Syu --noconfirm | |
| 20 | 20 | ||
| 21 | 21 | pacman -S --needed --noconfirm \ | |
| 22 | 22 | eza bat fd ripgrep fzf zoxide starship \ | |
| @@ -9,14 +9,14 @@ USER_HOME=$(getent passwd "$USERNAME" | cut -d: -f6) | |||||
| 9 | log() { echo "[cli-tools] $*" | tee -a "$LOG_FILE"; } | 9 | log() { echo "[cli-tools] $*" | tee -a "$LOG_FILE"; } | ||
| 10 | 10 | ||||
| 11 | if [[ -z "$USERNAME" ]]; then | 11 | if [[ -z "$USERNAME" ]]; then | ||
| 12 | - | log "ERROR: Could not detect user in /home" | 12 | + | log "ERROR: Could not detect user via getent passwd" |
| 13 | exit 1 | 13 | exit 1 | ||
| 14 | fi | 14 | fi | ||
| 15 | 15 | ||||
| 16 | log "Installing modern CLI toolkit for $USERNAME..." | 16 | log "Installing modern CLI toolkit for $USERNAME..." | ||
| 17 | 17 | ||||
| 18 | - | log "Refreshing package databases..." | 18 | + | log "Upgrading system and refreshing package databases..." |
| 19 | - | pacman -Sy --noconfirm | 19 | + | pacman -Syu --noconfirm |
| 20 | 20 | ||||
| 21 | pacman -S --needed --noconfirm \ | 21 | pacman -S --needed --noconfirm \ | ||
| 22 | eza bat fd ripgrep fzf zoxide starship \ | 22 | eza bat fd ripgrep fzf zoxide starship \ | ||
M install/mandatory/post-boot/dms-opt-in.sh +6 -2
| @@ -57,8 +57,12 @@ case "$REPLY" in | |||
| 57 | 57 | [yY][eE][sS]|[yY]) | |
| 58 | 58 | log "User opted in — starting Dank Linux installer." | |
| 59 | 59 | echo "Starting Dank Linux installer..." | |
| 60 | - | curl -fsSL https://install.danklinux.com | sh | |
| 61 | - | log "Dank Linux installation complete." | |
| 60 | + | if curl -fsSL https://install.danklinux.com | sh; then | |
| 61 | + | log "Dank Linux installation complete." | |
| 62 | + | else | |
| 63 | + | log "ERROR: Dank Linux installer failed (exit code $?)." | |
| 64 | + | exit 1 | |
| 65 | + | fi | |
| 62 | 66 | ;; | |
| 63 | 67 | *) | |
| 64 | 68 | log "User skipped Dank Linux installation." | |
| @@ -57,8 +57,12 @@ case "$REPLY" in | |||||
| 57 | [yY][eE][sS]|[yY]) | 57 | [yY][eE][sS]|[yY]) | ||
| 58 | log "User opted in — starting Dank Linux installer." | 58 | log "User opted in — starting Dank Linux installer." | ||
| 59 | echo "Starting Dank Linux installer..." | 59 | echo "Starting Dank Linux installer..." | ||
| 60 | - | curl -fsSL https://install.danklinux.com | sh | 60 | + | if curl -fsSL https://install.danklinux.com | sh; then |
| 61 | - | log "Dank Linux installation complete." | 61 | + | log "Dank Linux installation complete." |
| 62 | + | else | |||
| 63 | + | log "ERROR: Dank Linux installer failed (exit code $?)." | |||
| 64 | + | exit 1 | |||
| 65 | + | fi | |||
| 62 | ;; | 66 | ;; | ||
| 63 | *) | 67 | *) | ||
| 64 | log "User skipped Dank Linux installation." | 68 | log "User skipped Dank Linux installation." | ||
M internal/application/handlers/post_install_handler.go +1 -0
| @@ -62,6 +62,7 @@ func (h *PostInstallHandler) Handle(ctx context.Context, cmd commands.PostInstal | |||
| 62 | 62 | // Write Dank Linux flag file if user opted in | |
| 63 | 63 | if cmd.InstallDankLinux { | |
| 64 | 64 | if err := h.writeDankLinuxFlag(cmd.MountPoint); err != nil { | |
| 65 | + | result.ErrorDetail = fmt.Sprintf("Failed to write Dank Linux flag: %v", err) | |
| 65 | 66 | return result, fmt.Errorf("failed to write dank linux flag: %w", err) | |
| 66 | 67 | } | |
| 67 | 68 | result.TasksRun = append(result.TasksRun, "dank-linux-flag") | |
| @@ -62,6 +62,7 @@ func (h *PostInstallHandler) Handle(ctx context.Context, cmd commands.PostInstal | |||||
| 62 | // Write Dank Linux flag file if user opted in | 62 | // Write Dank Linux flag file if user opted in | ||
| 63 | if cmd.InstallDankLinux { | 63 | if cmd.InstallDankLinux { | ||
| 64 | if err := h.writeDankLinuxFlag(cmd.MountPoint); err != nil { | 64 | if err := h.writeDankLinuxFlag(cmd.MountPoint); err != nil { | ||
| 65 | + | result.ErrorDetail = fmt.Sprintf("Failed to write Dank Linux flag: %v", err) | |||
| 65 | return result, fmt.Errorf("failed to write dank linux flag: %w", err) | 66 | return result, fmt.Errorf("failed to write dank linux flag: %w", err) | ||
| 66 | } | 67 | } | ||
| 67 | result.TasksRun = append(result.TasksRun, "dank-linux-flag") | 68 | result.TasksRun = append(result.TasksRun, "dank-linux-flag") | ||
M internal/application/handlers/post_install_handler_test.go +1 -0
| @@ -177,6 +177,7 @@ func TestPostInstallHandler_Handle_WithDankLinux(t *testing.T) { | |||
| 177 | 177 | mockLogger.EXPECT().LogPath().Return("/var/log/archup-install.log").AnyTimes() | |
| 178 | 178 | mockFS.EXPECT().Exists(gomock.Any()).Return(false, nil).AnyTimes() | |
| 179 | 179 | mockFS.EXPECT().ReadFile(gomock.Any()).Return([]byte("graphics: yes"), nil).AnyTimes() | |
| 180 | + | mockFS.EXPECT().WriteFile(gomock.Eq("/mnt/var/lib/archup-install-danklinux"), gomock.Any(), gomock.Any()).Return(nil) | |
| 180 | 181 | mockFS.EXPECT().WriteFile(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() | |
| 181 | 182 | mockFS.EXPECT().MkdirAll(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() | |
| 182 | 183 | mockHTTP.EXPECT().Get(gomock.Any()).Return(newMockResponse(ctrl, http.StatusOK, []byte("content")), nil).AnyTimes() | |
| @@ -177,6 +177,7 @@ func TestPostInstallHandler_Handle_WithDankLinux(t *testing.T) { | |||||
| 177 | mockLogger.EXPECT().LogPath().Return("/var/log/archup-install.log").AnyTimes() | 177 | mockLogger.EXPECT().LogPath().Return("/var/log/archup-install.log").AnyTimes() | ||
| 178 | mockFS.EXPECT().Exists(gomock.Any()).Return(false, nil).AnyTimes() | 178 | mockFS.EXPECT().Exists(gomock.Any()).Return(false, nil).AnyTimes() | ||
| 179 | mockFS.EXPECT().ReadFile(gomock.Any()).Return([]byte("graphics: yes"), nil).AnyTimes() | 179 | mockFS.EXPECT().ReadFile(gomock.Any()).Return([]byte("graphics: yes"), nil).AnyTimes() | ||
| 180 | + | mockFS.EXPECT().WriteFile(gomock.Eq("/mnt/var/lib/archup-install-danklinux"), gomock.Any(), gomock.Any()).Return(nil) | |||
| 180 | mockFS.EXPECT().WriteFile(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() | 181 | mockFS.EXPECT().WriteFile(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() | ||
| 181 | mockFS.EXPECT().MkdirAll(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() | 182 | mockFS.EXPECT().MkdirAll(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() | ||
| 182 | mockHTTP.EXPECT().Get(gomock.Any()).Return(newMockResponse(ctrl, http.StatusOK, []byte("content")), nil).AnyTimes() | 183 | mockHTTP.EXPECT().Get(gomock.Any()).Return(newMockResponse(ctrl, http.StatusOK, []byte("content")), nil).AnyTimes() | ||
M internal/interfaces/tui/app.go +2 -0
| @@ -328,8 +328,10 @@ func (a *App) handleDankLinuxInput(msg tea.KeyMsg) (tea.Model, tea.Cmd) { | |||
| 328 | 328 | switch msg.String() { | |
| 329 | 329 | case "up", "shift+tab": | |
| 330 | 330 | a.dankLinuxModel.MoveUp() | |
| 331 | + | return a, nil | |
| 331 | 332 | case "down", "tab": | |
| 332 | 333 | a.dankLinuxModel.MoveDown() | |
| 334 | + | return a, nil | |
| 333 | 335 | case "enter": | |
| 334 | 336 | a.formData.InstallDankLinux = a.dankLinuxModel.SelectedOption().Value | |
| 335 | 337 | return a.startInstallation() | |
| @@ -328,8 +328,10 @@ func (a *App) handleDankLinuxInput(msg tea.KeyMsg) (tea.Model, tea.Cmd) { | |||||
| 328 | switch msg.String() { | 328 | switch msg.String() { | ||
| 329 | case "up", "shift+tab": | 329 | case "up", "shift+tab": | ||
| 330 | a.dankLinuxModel.MoveUp() | 330 | a.dankLinuxModel.MoveUp() | ||
| 331 | + | return a, nil | |||
| 331 | case "down", "tab": | 332 | case "down", "tab": | ||
| 332 | a.dankLinuxModel.MoveDown() | 333 | a.dankLinuxModel.MoveDown() | ||
| 334 | + | return a, nil | |||
| 333 | case "enter": | 335 | case "enter": | ||
| 334 | a.formData.InstallDankLinux = a.dankLinuxModel.SelectedOption().Value | 336 | a.formData.InstallDankLinux = a.dankLinuxModel.SelectedOption().Value | ||
| 335 | return a.startInstallation() | 337 | return a.startInstallation() | ||
M internal/interfaces/tui/models/form.go +1 -0
| @@ -311,6 +311,7 @@ func (fm *FormModelImpl) extractData() { | |||
| 311 | 311 | Timezone: fm.fields[4].Value(), | |
| 312 | 312 | Locale: fm.fields[5].Value(), | |
| 313 | 313 | Keymap: fm.fields[6].Value(), | |
| 314 | + | InstallDankLinux: fm.data.InstallDankLinux, | |
| 314 | 315 | } | |
| 315 | 316 | } | |
| 316 | 317 | ||
| @@ -311,6 +311,7 @@ func (fm *FormModelImpl) extractData() { | |||||
| 311 | Timezone: fm.fields[4].Value(), | 311 | Timezone: fm.fields[4].Value(), | ||
| 312 | Locale: fm.fields[5].Value(), | 312 | Locale: fm.fields[5].Value(), | ||
| 313 | Keymap: fm.fields[6].Value(), | 313 | Keymap: fm.fields[6].Value(), | ||
| 314 | + | InstallDankLinux: fm.data.InstallDankLinux, | |||
| 314 | } | 315 | } | ||
| 315 | } | 316 | } | ||
| 316 | 317 | ||||
M internal/shell/configurator.go +1 -1
| @@ -20,6 +20,6 @@ func (c *Configurator) Configure(username, userHome string) (ConfigResult, error | |||
| 20 | 20 | return ConfigResult{ | |
| 21 | 21 | ThemesApplied: 0, | |
| 22 | 22 | ThemesFailed: []string{}, | |
| 23 | - | Warnings: []string{}, | |
| 23 | + | Warnings: []string{"Shell configuration deferred to cli-tools.sh on first boot"}, | |
| 24 | 24 | }, nil | |
| 25 | 25 | } | |
| @@ -20,6 +20,6 @@ func (c *Configurator) Configure(username, userHome string) (ConfigResult, error | |||||
| 20 | return ConfigResult{ | 20 | return ConfigResult{ | ||
| 21 | ThemesApplied: 0, | 21 | ThemesApplied: 0, | ||
| 22 | ThemesFailed: []string{}, | 22 | ThemesFailed: []string{}, | ||
| 23 | - | Warnings: []string{}, | 23 | + | Warnings: []string{"Shell configuration deferred to cli-tools.sh on first boot"}, |
| 24 | }, nil | 24 | }, nil | ||
| 25 | } | 25 | } | ||