Skip to content
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.

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

Add warning if file cannot be managed by hjem #2

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DamitusThyYeetus123
Copy link

No description provided.

@yttrinis
Copy link

yttrinis commented Dec 19, 2024

following shell logic, the whole:

// lines 187-191

if [ "$code" -eq "0" ] ; then
 exit 0
else
  exit 1
fi

is pointless, just replace all with exit $code

also could replace all of those value.xxx with one value = { but that's personal preference...

@DamitusThyYeetus123
Copy link
Author

Oh yup, my bad, will fix those

@yttrinis
Copy link

you know what, now that am looking at it, since you are using [ and not bash's [[ you can even remove all of lines 178 - 192 for:

#!${pkgs.runtimeShell} -e
for var in "$@"; do
    [ -L "$var" ] || { echo "$var is not controlled by Hjem due to a file conflict"; exit 1; }
done
'';

modules/nixos.nix Outdated Show resolved Hide resolved
modules/nixos.nix Outdated Show resolved Hide resolved
modules/nixos.nix Outdated Show resolved Hide resolved
@DamitusThyYeetus123
Copy link
Author

Made some extra changes but forgot to commit them, will update with all this soon

modules/nixos.nix Outdated Show resolved Hide resolved
modules/nixos.nix Outdated Show resolved Hide resolved
@DamitusThyYeetus123
Copy link
Author

@eclairevoyant can you please remove your requested change? I have implemented it in other commits.

@eclairevoyant
Copy link
Member

None of my feedback appears to have been addressed yet

modules/nixos.nix Outdated Show resolved Hide resolved
modules/nixos.nix Outdated Show resolved Hide resolved
modules/nixos.nix Outdated Show resolved Hide resolved
modules/nixos.nix Outdated Show resolved Hide resolved
modules/nixos.nix Outdated Show resolved Hide resolved
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.

4 participants