Skip to content

Testing reviewer #2

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.

Sign up for GitHub

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

Open
brunobergher wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Testing reviewer #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
04196f5
FixeS
brunobergher Oct 13, 2025
873699e
Something
brunobergher Oct 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .zshrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ source ~/dotfiles/aliases/ls.sh
source ~/dotfiles/aliases/maintenance.sh
source ~/dotfiles/aliases/meta.sh
source ~/dotfiles/aliases/network.sh
source ~/dotfiles/aliases/servers.sh
source ~/dotfiles/aliases/scripts.sh
source ~/dotfiles/aliases/servers.sh
source ~/dotfiles/aliases/shortcuts.sh

# Utilities
Expand Down
2 changes: 1 addition & 1 deletion aliases/cli.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ alias ..="cd .."
alias ...="cd ../.."
alias ....="cd ../../.."
alias .....="cd ../../../.."
alias dev='cd ~/dev'
alias dev="cd ~/dev "
Copy link

@roo-code-bruno roo-code-bruno bot Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alias alias dev="cd ~/dev " in aliases/cli.sh contains a trailing space inside the quoted command. That trailing space becomes part of the executed command and can lead to unexpected behavior (the extra space will be passed to the shell when the alias expands). Remove the trailing space so it reads alias dev="cd ~/dev".

Copy link

@roo-code-bruno roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alias contains a trailing space inside the quoted command, which becomes part of the executed command and may cause unexpected behavior. Remove the trailing space so the alias expands cleanly.

Suggested change
alias dev="cd ~/dev "
alias dev="cd ~/dev"

Copy link

@roo-code-bruno roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alias contains a trailing space inside the quoted command, which becomes part of the executed command and can cause unexpected behavior. Remove the trailing space so the alias expands cleanly.

Suggested change
alias dev="cd ~/dev "
alias dev="cd ~/dev"


# Enable aliases to be sudo’ed
alias sudo='sudo '
Expand Down
3 changes: 3 additions & 0 deletions aliases/git.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,8 @@ alias gwr="git worktree remove"
alias gwl="git worktree list"
alias gup="git fetch origin && git merge origin/main"




git config --global user.name "Bruno Bergher"
Copy link

@roomote-v0 roomote-v0 bot Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The aliases file sets global Git config on shell startup by running git config --global user.name ... and git config --global user.email .... This will execute every time a shell is sourced and may unexpectedly modify global config or slow shell startup. Consider removing these lines from aliases/git.sh and placing them into bootstrap.sh behind a guard so they run once during bootstrap. Example:\n\nif ! git config --global user.name >/dev/null 2>&1; then\n git config --global user.name "Bruno Bergher"\nfi\nif ! git config --global user.email >/dev/null 2>&1; then\n git config --global user.email "me@brunobergher.com"\nfi\n

Copy link

@roo-code-bruno roo-code-bruno bot Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two lines at the end of aliases/git.sh run git config --global ... every time a shell starts because this file is sourced from ~/.zshrc. That can slow shell startup, repeatedly write global config, and unexpectedly overwrite a user's settings. Move these setup commands to the bootstrap/setup workflow (run once), or guard them with a conditional check (e.g. check git config --global --get user.name before setting) so they don't execute on every shell source.

Copy link

@roo-code-bruno roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is sourced on every shell startup via .zshrc, so running global Git config here will execute each time and may overwrite user preferences or slow startup. Move these to the bootstrap flow and guard them (e.g., only set if unset) so they run once.

git config --global user.email me@brunobergher.com
Comment on lines 21 to 22
Copy link

@roo-code-bruno roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running global Git config in an aliases file will execute on every shell startup (this file is sourced by .zshrc). This can repeatedly overwrite user preferences and adds overhead. Guard these calls (or move them to bootstrap.sh to run once during setup).

Suggested change
git config --global user.name "Bruno Bergher"
git config --global user.email me@brunobergher.com
if ! git config --global user.name >/dev/null 2>&1; then
git config --global user.name "Bruno Bergher"
fi
if ! git config --global user.email >/dev/null 2>&1; then
git config --global user.email "me@brunobergher.com"
fi

34 changes: 10 additions & 24 deletions bootstrap.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,13 @@
#!/usr/bin/env bash

echo "Bootstrapping dotfiles…";
cd "$(dirname "${BASH_SOURCE}")";

git pull origin main;

function doIt() {
rsync --exclude ".git/" \
--exclude ".DS_Store" \
--exclude ".osx" \
--exclude "bootstrap.sh" \
--exclude "README.md" \
--exclude "LICENSE-MIT.txt" \
-avh --no-perms . ~;
source ~/.bash_profile;
}

if [ "$1" == "--force" -o "$1" == "-f" ]; then
doIt;
else
read -p "This may overwrite existing files in your home directory. Are you sure? (y/n) " -n 1;
echo "";
if [[ $REPLY =~ ^[Yy]$ ]]; then
doIt;
fi;
fi;
unset doIt;
rsync --exclude ".git/" \
Copy link

@roomote-v0 roomote-v0 bot Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated bootstrap.sh removes the interactive confirmation and the --force/-f guard, and now runs rsync non-interactively with its output redirected to /dev/null. This is a behavioral change that can silently overwrite user files and hide errors. Recommend restoring an explicit confirmation or a --yes/--force flag, and avoid redirecting rsync output to /dev/null so failures are visible. Also sourcing ~/.zshrc from a Bash script can have unintended side-effects; consider using exec zsh -lc 'source ~/.zshrc' or instructing the user to restart their shell instead.

Suggested minimal actions:

  • Reintroduce a --force / prompt flow so the default is safe.
  • Remove > /dev/null so rsync errors surface (or redirect only stdout, keep stderr visible).
  • Do not source ~/.zshrc from bash; either spawn zsh or ask the user to restart their shell.

Copy link

@roo-code-bruno roo-code-bruno bot Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This updated bootstrap.sh removes the interactive confirmation and the doIt wrapper, making the script run rsync non-interactively and silencing its output by redirecting to /dev/null. Two issues: 1) Removing the confirmation increases the risk of accidental overwrites when a user runs this script. 2) Redirecting output to /dev/null hides rsync errors and progress that could help diagnose failures. Recommend restoring a safe interactive prompt (or --force flag) and keeping stderr at least (e.g. > /dev/null 2>&1 -> > /dev/null is worse; instead consider --quiet or redirect only stdout while leaving stderr), or document that this script is destructive and will run without prompt.

Copy link

@roo-code-bruno roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change removes the interactive confirmation/--force guard, which makes the bootstrap destructive by default. Reintroduce a prompt or a --force flag so accidental overwrites are avoided by default.

Copy link

@roo-code-bruno roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the interactive confirmation/--force guard makes this script destructive by default and increases risk of accidental overwrites. Reintroduce a prompt or a --force flag so the default path is safe.

--exclude ".DS_Store" \
--exclude ".osx" \
--exclude "bootstrap.sh" \
--exclude "README.md" \
--exclude "LICENSE-MIT.txt" \
-avh --no-perms . ~ > /dev/null;
Copy link

@roo-code-bruno roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redirecting rsync output to /dev/null hides failures and makes troubleshooting harder. Keep errors visible (or avoid silencing altogether) so problems surface during bootstrap.

Suggested change
-avh --no-perms . ~ > /dev/null;
-avh --no-perms . ~;

Copy link

@roo-code-bruno roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redirecting rsync output to /dev/null hides failures and makes troubleshooting harder. Keep errors visible (or avoid silencing altogether) so problems surface during bootstrap.

source ~/.zshrc;
Copy link

@roo-code-bruno roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcing ~/.zshrc from a non-interactive bash script can cause unintended side effects and mixes shells. Prefer not to source it here; ask the user to start a new terminal session (or, if absolutely needed, exec zsh in a controlled subshell).

Copy link

@roo-code-bruno roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcing ~/.zshrc from a non-interactive bash script can cause unintended side effects and mixes shells. Prefer not to source it here; ask the user to restart their shell (or exec zsh in a controlled subshell). Replacing this line with a comment avoids side effects.

Suggested change
source ~/.zshrc;
# Do not source ~/.zshrc here; restart your terminal session instead

echo "Done. Start a new terminal session to see the changes.";