Testing reviewer by brunobergher · Pull Request #2 · brunobergher/dotfiles

brunobergher · 2025-10-13T16:34:05Z

No description provided.

roomote-v0 · 2025-10-13T16:38:29Z

aliases/git.sh

+
+
+
 git config --global user.name "Bruno Bergher"


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

roomote-v0 · 2025-10-13T16:39:07Z

bootstrap.sh

-	fi;
-fi;
-unset doIt;
+rsync --exclude ".git/" \


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.

roo-code-bruno · 2025-10-13T16:39:50Z

aliases/git.sh

+
+
+
 git config --global user.name "Bruno Bergher"


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.

roo-code-bruno · 2025-10-13T16:40:58Z

aliases/cli.sh

 alias ....="cd ../../.."
 alias .....="cd ../../../.."
-alias dev='cd ~/dev'
+alias dev="cd ~/dev "


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".

roo-code-bruno · 2025-10-13T16:41:49Z

bootstrap.sh

-	fi;
-fi;
-unset doIt;
+rsync --exclude ".git/" \


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.

roo-code-bruno · 2025-10-17T09:23:33Z

Review Summary

I've reviewed the changes in this PR. Here are the issues that need to be addressed:

Remove git config commands from aliases/git.sh that run on every shell startup (lines 21-22) - move to bootstrap or add conditional guards
Remove trailing space from dev alias in aliases/cli.sh (line 6)
Restore safety features in bootstrap.sh: add interactive confirmation or --force flag, and don't redirect rsync output to /dev/null
Don't source ~/.zshrc from bash script in bootstrap.sh - either spawn zsh or instruct user to restart shell

Please address these issues before merging.

roo-code-bruno · 2025-10-20T14:08:58Z

PR Review TODOs:

Fix trailing space in alias: aliases/cli.sh
Move global Git config out of sourced aliases and guard in bootstrap: aliases/git.sh, aliases/git.sh
Suggested guard:

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

Avoid hiding rsync output/errors: remove redirection at bootstrap.sh
Do not source ~/.zshrc from a bash script: bootstrap.sh; prefer asking the user to open a new shell or exec zsh explicitly
Reintroduce a confirmation or --force flag so sync is safe by default: bootstrap.sh

roo-code-bruno · 2025-10-20T14:14:41Z

aliases/cli.sh

 alias ....="cd ../../.."
 alias .....="cd ../../../.."
-alias dev='cd ~/dev'
+alias dev="cd ~/dev "


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"

roo-code-bruno · 2025-10-20T14:16:16Z

aliases/git.sh

+
+
+
 git config --global user.name "Bruno Bergher"


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.

roo-code-bruno · 2025-10-20T14:18:08Z

bootstrap.sh

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


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 . ~;

roo-code-bruno · 2025-10-20T14:18:39Z

bootstrap.sh

+	--exclude "README.md" \
+	--exclude "LICENSE-MIT.txt" \
+	-avh --no-perms . ~ > /dev/null;
+source ~/.zshrc;


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).

roo-code-bruno · 2025-10-20T14:19:55Z

bootstrap.sh

-	fi;
-fi;
-unset doIt;
+rsync --exclude ".git/" \


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.

roo-code-bruno · 2025-10-20T14:22:05Z

PR review TODOs for #2

Remove trailing space in dev alias so the command expands cleanly in aliases/cli.sh
Stop running global Git config on every shell startup; move or guard the commands in aliases/git.sh (prefer moving to bootstrap.sh with conditional checks so they run once)
Keep rsync errors visible; avoid silencing output by removing redirection in bootstrap.sh
Do not source ~/.zshrc from a non-interactive bash script; instruct the user to restart their shell (or exec zsh) in bootstrap.sh
Reintroduce an explicit confirmation or a --force flag to avoid destructive defaults in bootstrap.sh

roo-code-bruno · 2025-10-20T14:25:21Z

aliases/cli.sh

 alias ....="cd ../../.."
 alias .....="cd ../../../.."
-alias dev='cd ~/dev'
+alias dev="cd ~/dev "


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"

roo-code-bruno · 2025-10-20T14:26:51Z

aliases/git.sh

 git config --global user.name "Bruno Bergher"
 git config --global user.email me@brunobergher.com


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

roo-code-bruno · 2025-10-20T14:28:23Z

bootstrap.sh

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


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

roo-code-bruno · 2025-10-20T14:30:09Z

bootstrap.sh

+	--exclude "README.md" \
+	--exclude "LICENSE-MIT.txt" \
+	-avh --no-perms . ~ > /dev/null;
+source ~/.zshrc;


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

roo-code-bruno · 2025-10-20T14:30:37Z

bootstrap.sh

-	fi;
-fi;
-unset doIt;
+rsync --exclude ".git/" \


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.

roo-code-bruno · 2025-10-22T10:55:04Z

Review Summary

I've reviewed the changes in this PR and identified the following issues that should be addressed:

Issues to Fix

Trailing space in dev alias - Remove the trailing space in alias dev="cd ~/dev " at aliases/cli.sh:6
Git config executed on every shell startup - Move git config commands from aliases/git.sh:21-22 to bootstrap.sh with guards to prevent repeated execution
Removed interactive confirmation - Restore the interactive confirmation or --force flag in bootstrap.sh to prevent accidental overwrites
Rsync output hidden - Remove the > /dev/null redirection at bootstrap.sh:11 to make errors visible
Sourcing ~/.zshrc from bash - Remove or replace the source ~/.zshrc line at bootstrap.sh:12 to avoid shell mixing issues

Follow Along on Roo Code Cloud

roo-code-bruno

Review complete. Found 5 issues that should be addressed before merging.

brunobergher added 2 commits October 13, 2025 14:07

FixeS

04196f5

Something

873699e

roomote-v0 bot reviewed Oct 13, 2025

View reviewed changes

roo-code-bruno bot reviewed Oct 13, 2025

View reviewed changes

roo-code-bruno bot reviewed Oct 20, 2025

View reviewed changes

roo-code-bruno bot reviewed Oct 22, 2025

View reviewed changes

		git config --global user.name "Bruno Bergher"
		git config --global user.email me@brunobergher.com No newline at end of file

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

Conversation

brunobergher commented Oct 13, 2025

Uh oh!

roomote-v0 bot Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

roomote-v0 bot Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

roo-code-bruno bot Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

roo-code-bruno bot Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

roo-code-bruno bot Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

roo-code-bruno bot commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review Summary

Uh oh!

roo-code-bruno bot commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

roo-code-bruno bot commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

roo-code-bruno bot Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

roo-code-bruno bot commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review Summary

Issues to Fix

Uh oh!

roo-code-bruno bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

roo-code-bruno bot commented Oct 17, 2025 •

edited

Loading

roo-code-bruno bot commented Oct 20, 2025 •

edited

Loading

roo-code-bruno bot commented Oct 20, 2025 •

edited

Loading

roo-code-bruno bot commented Oct 22, 2025 •

edited

Loading