Support querying environment variable presence/absence#454
Support querying environment variable presence/absence#454Gabriella439 wants to merge 1 commit intomainfrom
Conversation
9999years
left a comment
There was a problem hiding this comment.
Looks good! Two comments:
- The names don't include
envor anything like it, which is a little ambiguous, especially because they take aText. I could imagine readingisPresent "foo"and thinking it's testing if a file exists. - I think
isSetandisUnsetare potential footguns — in my experience, it's very rare to consider a variable 'set' if it exists but is set to the empty string. We might consider adding a note to this effect to the documentation. ("In most cases, you wantisPresent.)
|
I'm happy to add an "env" or something to the names to address (1) so I'll do that. I'm a bit surprised about (2) because I had the opposite point of view (that |
|
Hmm, that's true. In my experience most environment variables are used for simple toggles. I would also expect the program to do some validation if the environment variable has semantics (e.g. check that a path exists if it's supposed to represent a path, etc.). |
Fixes #452