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

Update install.md #1889

Merged
merged 5 commits into from
Feb 15, 2024
Merged

Update install.md #1889

merged 5 commits into from
Feb 15, 2024

Conversation

ckuhtz
Copy link
Contributor

@ckuhtz ckuhtz commented Feb 14, 2024

the modified commands only work as root (can't chown otherwise) and this version should work as anyone: Wouldn't it be easier to run the container as a specific user, namely whoever is the current user?

Also replaced pwd fork for ${PWD}.

Suggestion for the future: the whole mkdir converted is really ugly IMHO; might be better to append a suffix to the file instead.

ckuhtz and others added 5 commits February 14, 2024 15:58
the modified commands only work as root (can't chown otherwise) and this version should work as anyone:  Wouldn't it be easier to run the container as a specific user, namely whoever is the current user?  

Also replaced pwd fork for ${PWD}.

Suggestion for the future: the whole mkdir converted is really ugly IMHO; might be better to append a suffix to the file instead.
bug fix :) removed ${GID}.
$() is required as the eval has to happen at runtime. modified.
@hellt
Copy link
Member

hellt commented Feb 15, 2024

good catch
converted is not needed anymore, I removed it entirely

@hellt hellt merged commit 49fd647 into srl-labs:main Feb 15, 2024
18 checks passed
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