Page MenuHomePhabricator

[direnv] Redirect `use flake` to `/dev/null` if `COMM_NIX_LOGGING_LEVEL` is "none"

Authored by atul on Sun, Mar 12, 7:05 PM.
Referenced Files
F445720: D7052.id23657.diff
Sun, Mar 26, 2:45 AM
F445625: D7052.id23734.diff
Sun, Mar 26, 12:10 AM
Unknown Object (File)
Sat, Mar 25, 7:25 AM
Unknown Object (File)
Fri, Mar 24, 6:33 PM
Unknown Object (File)
Tue, Mar 21, 5:32 PM
Unknown Object (File)
Sat, Mar 18, 6:19 AM
Unknown Object (File)
Sat, Mar 18, 6:19 AM
Unknown Object (File)
Sat, Mar 18, 6:19 AM



Found the logging to be kind of annoying. Would prefer if Nix/Direnv worked completely in the background. Realize this is a personal preference so this change won't affect anyone unless they explicitly set COMM_NIX_LOGGING_LEVEL="none".

Test Plan


1e2bae.png (468×1 px, 342 KB)

After setting COMM_NIX_LOGGING_LEVEL="none":

c0c660.png (110×866 px, 58 KB)

Diff Detail

rCOMM Comm
Lint Not Applicable
Tests Not Applicable

Event Timeline

atul published this revision for review.Sun, Mar 12, 7:07 PM
atul edited the test plan for this revision. (Show Details)
atul added inline comments.
11–15 ↗(On Diff #23657)

I don't know bash so let me definitely know if there's something I'm missing or there's an obviously better way to do this

11–16 ↗(On Diff #23657)

bash doesn't have a set "boolean" value pair. Generally you see people setting a variable to nothing VAR= or unset VAR for false values; and setting it to anything else to be true VAR=1

ashoat removed a reviewer: ashoat. ashoat added 1 blocking reviewer(s): jon.
11–16 ↗(On Diff #23657)

Ah was hoping for this to be more of an "enum" than a "boolean." As in we might have "none", "error", "verbose", etc. in the future?

I wanted wanted to maintain the default behavior so I didn't want COMM_NIX_LOGGING_LEVEL being unset to disable logs. I wanted disabled logs to be opt-in, so I set COMM_NIX_LOGGING_LEVEL to "something" (in this case "none") in order to toggle that.

atul added a reviewer: varun. atul removed 1 blocking reviewer(s): jon.Tue, Mar 14, 9:15 AM
11–16 ↗(On Diff #23657)

Then I would invert the intention:


jon’s suggestion makes sense to me

This revision is now accepted and ready to land.Tue, Mar 14, 10:07 AM
11–16 ↗(On Diff #23657)

gotcha, will update this diff and use SUPPRESS_COMM_NIX_LOGGING as the environment variable instead

atul edited the test plan for this revision. (Show Details)

address feedback

This revision was landed with ongoing or failed builds.Tue, Mar 14, 1:08 PM
This revision was automatically updated to reflect the committed changes.