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.

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

Vote on namespace for args parser #1224

Closed
mhdawson opened this issue May 4, 2022 · 5 comments · Fixed by #1225
Closed

Vote on namespace for args parser #1224

mhdawson opened this issue May 4, 2022 · 5 comments · Fixed by #1225

Comments

@mhdawson
Copy link
Member

mhdawson commented May 4, 2022

There was agreement today in the TSC meeting that we should start a vote on the question of which namespace parseArgs should go under as being discussed in nodejs/node#42675. Vote propsosed by @Trott and seconded by @mhdawson.

Since we did not have enough people in the meeting, this issue is to give other TSC members an FYI and time to comment/object.

The plan is to start the vote this Friday if there are no objections before then. @aduh95 volunteered to spin up the vote using his great tool.

The question for the vote is:

Should parseArgs be added under

  1. process
  2. util
@mhdawson
Copy link
Member Author

mhdawson commented May 4, 2022

@nodejs/tsc FYI

@Jamesernator
Copy link

Jamesernator commented May 5, 2022

I personally think util makes more sense given args is configurable. Like:

const args = process.parseArgs({
   args: ["--some", ...otherArgs],
});

just doesn't seem right for something that has nothing to do with the current process. The inversion seems okay, a utility (util) that happens to default to the current process.

For something comparable, path.resolve(...) uses the current process.cwd() as the default root, so it's not new terroritory for Node APIs to default to process-level values when more specific choices aren't given.

@RaisinTen
Copy link
Contributor

In most cases however, I wouldn't expect people to configure args. So my understanding is that process.parseArgs() will effectively replace most uses of process.argv, so parseArgs() should belong to process because of the same reason process.argv belongs there IMO.

For something comparable, path.resolve(...) uses the current process.cwd() as the default root, so it's not new terroritory for Node APIs to default to process-level values when more specific choices aren't given.

Yea, I think path is a better module for resolve() than util because most of the APIs in util are almost like JS language features and I don't think resolving a path falls into that category.

@ljharb
Copy link
Member

ljharb commented May 5, 2022

If anything, imo path.resolve is a good argument for process, because it's a solution that lives close to its problem domain, instead of in a "util grab bag because we didn't have a better option" place.

@aduh95
Copy link
Contributor

aduh95 commented May 5, 2022

Since it looks like there's consensus on what are the candidates for this vote, I went ahead and opened the voting PR: #1225

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants