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

mv .envrc → .envrc.example #178

Closed
wants to merge 1 commit into from
Closed

Conversation

toastal
Copy link
Contributor

@toastal toastal commented Apr 1, 2024

It isn’t a good idea to make a personal configuration file hard to work with since it’s being tracked by the repository. This will a) trigger direnv to fire off its allow script before a use gets a chance to see what arbitrary code is about to be executed on their behalf & b) make making modifications more difficult as it would be easy to accidentally commit those modifications or requires addings all sorts of unnecessary extra configuration to allow a safe place to put in custom environment variables.

The simple solution is to just leave an example for those that want it.

It isn’t a good idea to make a personal configuration file hard to work
with since it’s being tracked by the repository. This will a) trigger
``direnv`` to fire off its allow script before a use gets a chance to
see what arbitrary code is about to be executed on their behalf & b) make
making modifications more difficult as it would be easy to accidentally
commit those modifications *or* requires addings all sorts of
unnecessary extra configuration to allow a safe place to put in custom
environment variables.

The simple solution is to just leave an example for those that want it.
@Janik-Haag
Copy link
Member

Janik-Haag commented Apr 1, 2024

We already had this discussion and the overall sentiment seems to be that the way it's currently handle is fine. #118 (comment)

@piegamesde
Copy link
Member

We discussed this in the team after you brought it up last time, and remain unconvinced that your proposed change is a net positive on the intended developer experience.

@piegamesde piegamesde closed this Apr 1, 2024
@toastal
Copy link
Contributor Author

toastal commented Apr 1, 2024

I couldn’t re-find the discussion nor did I see the follow-up conversation. Following the link, there never was a follow-up conversation posted.

@toastal toastal deleted the mv-envrc branch April 1, 2024 09:39
@piegamesde
Copy link
Member

Yes, we forgot to do that unfortunately.

@infinisil
Copy link
Member

I'd be okay with something like this though:

if [[ -f .envrc.local ]]; then
  source .envrc.local
else
  use_nix
fi

This will allow you to customise the behavior without changing the default.

@dasJ
Copy link
Member

dasJ commented Apr 2, 2024

You wouldn't: direnv/direnv#348 (comment)

@mweinelt mweinelt mentioned this pull request Jul 9, 2024
13 tasks
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.

5 participants