feat: add BAZEL env variable to js_binary#1351
Conversation
a05386b to
3c7ed2f
Compare
alexeagle
left a comment
There was a problem hiding this comment.
Did you search for a conventional variable set in other languages? What do ppl do there?
696fe59 to
d636729
Compare
If you take the CI vendors & tools as an example, they all set https://docs.github.com/en/actions/learn-github-actions/variables#default-environment-variables https://buildkite.com/docs/pipelines/environment-variables#buildkite-environment-variables https://circleci.com/docs/variables/#built-in-environment-variables Setting |
db6c713 to
c4f5663
Compare
|
I haven't seen the |
c4f5663 to
575aa48
Compare
|
Bazel itself sets |
Yes, tho There is precedent for setting |
|
I'm just nitpicking this because every added variable has the potential to collide with something the user already set, and shorter is more likely. |
I do check if it is already set and don't set override it if it is. Perhaps I should create an issue on bazelbuild/bazel so that bazel itself sets |
|
A bit related: bazelbuild/bazel#15470 |
|
Greg Magolan seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Differentiating between Bazel and non-Bazel environments is very common and to date users have been using either
BAZEL_TESTorJS_BINARY__*env vars. The former is not set by bazel underbazel runand the latter is not particularly ergonomic.BAZELis simpler and easier to remember.Type of change
Test plan