Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Propose vote to unblock --env-file PRs #1577

Merged
merged 4 commits into from
Jun 17, 2024
Merged

Propose vote to unblock --env-file PRs #1577

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8f5ec5b
Propose vote to unblock `--env-file` PRs
aduh95 Jun 14, 2024
16cdc0e
Apply suggestions from code review
aduh95 Jun 14, 2024
0ba834b
Apply suggestions from code review
aduh95 Jun 14, 2024
d435ed8
Apply suggestions from code review
aduh95 Jun 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions votes/initiateNewVote/_EDIT_ME.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

# 1. Select a subject for the vote. This can be a question addressed to the TSC
# voting members.
subject: REPLACEME
subject: How should `node` handle non-existent `.env` files passed via CLI flags?

# 2. You can leave the header instructions as is, or modify them if you see fit.
headerInstructions: |
Expand All @@ -24,8 +24,14 @@ headerInstructions: |
# voters express their preference for each candidates, no matter how many
# there are.
candidates:
- TODO
- TODO
- Throw when passed to `--env-file` flag, provide no other flag (status quo).
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
- Ignore when passed to `--env-file` flag, provide no other flag.
- Warn when passed to `--env-file` flag, provide no other flag.
- Throw when passed to `--env-file` flag, warn when passed to a new `--env-file-if-exists`.
- Throw when passed to `--env-file` flag, ignore when passed to a new `--env-file-if-exists`.
- Ignore when passed to `--env-file` flag, throw when passed to a new `--env-file-must-exist`.
- Throw when passed to `--env-file` flag, unless new `--env-file-when-missing=warn` is also passed.
- Warn when passed to `--env-file` flag, unless new `--env-file-when-missing=throw` is also passed.
Comment on lines +33 to +34
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think anyone has been advocating for these. These options would mean you can't have a mix of required and optional files to load.

Copy link
Contributor Author

@aduh95 aduh95 Jun 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think anyone has been advocating for these.

I have (see nodejs/node#53060 (comment))

These options would mean you can't have a mix of required and optional files to load.

That's correct

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I missed that. I feel like a bunch of people would be upset if this is what we land, because the use case they want to support is “I have some env files that every environment needs, plus another env file that only localhost needs”. That was what prompted --env-file-optional if I remember correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like a bunch of people would be upset if this is what we land

Yes probably, and that applies to all the candidates BTW – that's why we're having a vote, because we couldn't find a solution that would please everybody.


# 4. Pass the following to false if it's important to keep the candidates in the
# order you define above. Presenting candidates in a fixed order tends to
Expand All @@ -35,11 +41,16 @@ canShuffleCandidates: true
# 5. Insert here a short description of the vote objectives and link to the
# issue it was discussed on to give the full context.
footerInstructions: |
TBD
Based on the result of this vote, we can unblock either https://github.com/nodejs/node/pull/53177,
https://github.com/nodejs/node/pull/53060, or neither if the status quo prevails. The name of the
proposed flags are non-binding, and are provided only as examples. If a proposal that includes a
new flag is accepted, the name of that flag can be bike-shed per the PR review process.
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
"Ignore" means "do not throw and do not warn" in this context.

# 6. Optionally, insert a brief introduction for the vote PR, in the markdown format.
prBody: |
The vote should stay open for 7 days, or until all TSC voting members have cast a ballot.

# 7. Optionally, choose an id that will be used for the branch name as well as
# the vote folder name. If not supplied, a UUID will be used.
path-friendly-id: null
path-friendly-id: how-to-handle-missing-env-files