Skip to content

fix: replace TODO with nginx:alpine image in svc-new.sh - #141

Closed
dsaikou618 wants to merge 5 commits into
masterfrom
fix/svc-new-image-todo
Closed

dsaikou618 wants to merge 5 commits into
masterfrom
fix/svc-new-image-todo

Conversation

@dsaikou618

Copy link
Copy Markdown
Collaborator

Replace the TODO placeholder with nginx:alpine as the default image in svc-new.sh

@dsaikou618
dsaikou618 requested a review from TriTacLe as a code owner October 27, 2026 10:27
@TriTacLe

Copy link
Copy Markdown
Owner

The nginx:alpine default (53e5c11) is a clean, correct fix for the TODO.

The other commit on this branch (17d5906, "config.sh enhancements for multi-user support") is not - it appends a hardcoded personal [user] block to the end of the shared, tracked shared/stow/git/.gitconfig:

[user]
    name = Saikou Amed Thidiane Diallo
    email = 323202163+dsaikou618@users.noreply.github.com

Git config takes the last matching value for a scalar key, so this literal block overrides the [include] path = ~/.gitconfig.local line above it for every single person who clones this repo and stows git - every commit from every user gets authored as this hardcoded identity instead of their own. It also commits a real name and GitHub-linked email into a shared tracked file, the opposite of what the profile system is supposed to keep local. That commit also duplicates/diverges from the profile.env and config.sh changes already in #139 rather than building on them - probably worth dropping entirely and rebasing just the nginx fix onto master directly.

This PR is also stacked on #139/#140's unmerged branches, so the diff includes their open issues too.

Copy link
Copy Markdown
Owner

Not mergeable as it stands, and the reason is the branch point, not the idea

The profiles feature is good work. link_profile_file refusing to clobber a real file, the ^[A-Za-z0-9._-]+$ check on the profile name, saving and restoring the selection around the source so a profile cannot rename itself, and making a failed submodule fetch non-fatal in install.sh are all the right calls, and the comments match the repo's voice.

The problem is that this branch was cut from an older master, so the diff silently reverts newer work. Four reverts, in order of how much they would hurt:

1. It replaces the repo owner's git identity with yours. shared/stow/git/.gitconfig loses

[user]
	name = TriTacLe
	email = tritac.le@gmail.com

from the top and gains

[user]
	name = Saikou Amed Thidiane Diallo
	email = 323202163+dsaikou618@users.noreply.github.com

at the bottom. Every machine installing these dotfiles would then commit as you. This also defeats the feature's own purpose: identity is exactly what should come from profiles/<name>/gitconfig.local, not from the shared file. Neither name belongs there.

2. It removes the NTNU setup. shared/stow/git/.config/git/ntnu.gitconfig is deleted, along with the commented includeIf "gitdir:~/Desktop/ntnu/" block and the [credential "https://git.ntnu.no"] helper. That breaks pushing to git.ntnu.no.

3. It deletes the STM32 filetype mapping in shared/stow/nvim/.config/nvim/lua/config/options.lua:

vim.filetype.add({ pattern = { [".*%.c[1-6]"] = "c" } })

That is live coursework config for main.c1..main.c6.

4. It reverts stow-pkg claude-config in shared.zsh from the current one-line submodule symlink back to source config.sh; link_claude_config, and flips alias x from claude to agy while adding agyy. The alias change is a personal preference being pushed into shared config.

Separately, .gitignore adds shared/stow/nvim/.config/nvim/lazy-lock.json. Untracking a lockfile is a real decision and unrelated to profiles; it should be its own change with its own argument.

Also: three PRs, one body of work

#139, #140 and #141 all share the same two base commits (e7c5e231, 3b875287). #141 contains #140's commit. #140's only unique contribution, the verify-system.sh shebang filter, was already fixed and merged in #138, so that part is now obsolete. All three report mergeable_state: dirty against current master, conflicting in arch/install-arch.sh, shared/scripts/verify-system.sh and shared/stow/scripts/.local/bin/token-rollup.py, which are the files #136 and #138 just touched.

Closing all three in favour of one re-cut branch.

What a re-cut should contain

Branch from current master and include only:

  • shared/scripts/config.sh: profile resolution, link_profile_file, bootstrap_profile
  • profiles/README.md and profiles/example/
  • .gitignore: the profiles/* block only
  • justfile: the profile target
  • install.sh: the non-fatal submodule guard
  • server/scripts/svc-new.sh: nginx:alpine (fine on its own, and it could equally go in as a one-line PR today)

Leave untouched: .gitconfig, ntnu.gitconfig, options.lua, shared.zsh, verify-system.sh, and the macOS ghostty/Brewfile/shader changes, which are unrelated to profiles.

One open question that needs the repo owner's answer before the re-cut, which is also why I am not synthesising the replacement PR myself: master currently links ~/.claude with a direct ln -sfn to the submodule, while this feature routes it through link_claude_config so a profile can point at its own Claude config repo. Both are defensible. That decision belongs to @TriTacLe, and the re-cut should follow whichever he picks.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants