Page MenuHomePhabricator

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

Authored by atul on Mar 12 2023, 7:05 PM.
Referenced Files
F1863414: D7052.id23733.diff
Sun, May 26, 6:00 AM
Unknown Object (File)
Sat, May 25, 2:08 PM
Unknown Object (File)
Sat, May 25, 2:08 PM
Unknown Object (File)
Fri, May 10, 7:20 PM
Unknown Object (File)
Mon, May 6, 9:03 PM
Unknown Object (File)
Wed, May 1, 4:21 PM
Unknown Object (File)
Wed, May 1, 3:14 PM
Unknown Object (File)
Sun, Apr 28, 7:50 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.Mar 12 2023, 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.Mar 14 2023, 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.Mar 14 2023, 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.Mar 14 2023, 1:08 PM
This revision was automatically updated to reflect the committed changes.