Conversation
Use common style
`--set FOO ""` is not strictly equivalent to `--unset FOO`. In the former case the environment variable still exists with an empty string as a value.
|
@zimbatm No, neither of those are correct. It's checking whether the variable is set, not for whether it's empty. |
|
@dezgeg I misread, the reverse of |
|
No. Correct: Wrong: |
|
Usually I treat the empty variable as unset. Otherwise it becomes hard to run bash scripts with |
|
Well here you can't, which was the point. |
| @@ -1,86 +1,99 @@ | |||
| #!bash | |||
There was a problem hiding this comment.
That shebang doesn't conform to POSIX. The interpreter must be an absolute path.
There was a problem hiding this comment.
And besides, there's no point in having a shebang because this script doesn't so anything. All it does is define functions, so it must necessarily be sourced. Doing "./make-wrapper.sh" makes no sense.
There was a problem hiding this comment.
Actually it's a hack so that ShellCheck and vim detect the proper shell dialect. Maybe there is a better way to do it.
There was a problem hiding this comment.
There was a problem hiding this comment.
Well, the shebang is pointless here. I guess running ShellCheck with "-s bash" would be better in this case. I certainly don't want a shebang that violates POSIX.
There was a problem hiding this comment.
It doesn't bother me that much because in that context we only need internal compatibility but I understand, I'll remove it.
|
@dezgeg for the rare cases where you specifically need to check the existence of a variable I think that |
|
Alright, abandoning this |
Things done:
nix-build --option build-use-chroot trueor nix.useChroot on NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)Replaces #13542