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

RFC: npm fund depth #152

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

fishcharlie
Copy link

@fishcharlie fishcharlie commented May 24, 2020

Formatted RFC

What / Why

This RFC proposes adding a depth flag to the npm fund command.

References

@fishcharlie
Copy link
Author

This is my first time submitting an RFC to npm 🎉. 🤞 that I did it correctly. However if I didn't, please let me know and I'll make any corrections needed.

I look forward to discussing this RFC and answering any questions that arise with the community!


Drawbacks:

- Funding bloat happens quickly. It's impossible to fund all dependencies, and users are more likely to fund direct dependencies since they know how they interact with them and utilize them directly.
Copy link
Contributor

Choose a reason for hiding this comment

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

what happens if one of your direct deps has no funding info, but it depends on dozens of packages that do declare funding info?

Copy link
Author

Choose a reason for hiding this comment

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

@ljharb In this case, if you set depth=0 I believe it should not output the sub-dependencies funding information. It should be strictly limited to printing funding information of your direct dependencies.

I wish there was an existing example of that behavior that we could emulate, however I can't think of a similar example for this case.

Do you think I should clarify that more in the document?

accepted/0000-npm-fund-depth.md Outdated Show resolved Hide resolved
Co-authored-by: Jordan Harband <ljharb@gmail.com>
@ruyadorno ruyadorno added the Agenda will be discussed at the Open RFC call label May 27, 2020
@darcyclarke darcyclarke removed the Agenda will be discussed at the Open RFC call label Jun 17, 2020
@ruyadorno ruyadorno added the Agenda will be discussed at the Open RFC call label Aug 26, 2020
@ruyadorno
Copy link
Contributor

hi @fishcharlie, first of all thanks for the contribution 😊 I'm going to leave here the feedback we got from the last time we discussed it (as we went over it again during this week's npm OpenRFC call).

In most of npm subcommands, we're shifting the focus away from the --depth flag in favor of using the --all flag. The --all flag is now supported in the npm ls as of npm7 beta as well as in npm outdated (and maybe some other cmds I don't recall at the moment).

There were a bunch of discussions when we first set to remove support to --depth in npm outdated in a previous RFC that you can check in case you want to dig more on the rationale there: #133

An important change @isaacs noticed is that we'd still want to preserve the default behavior of npm fund printing funding info of your entire install tree, so the RFC should also mention that the all config would now need a different default value (current is false). This new default value of all should now be null allowing the subcommands to override it with their expected default values (which will be false for ls and outdated and true for fund).

Next action item here now is to tweak the RFC to reflect these proposed changes 😄

@ruyadorno ruyadorno removed the Agenda will be discussed at the Open RFC call label Aug 26, 2020
@settings settings bot removed the Proposal label Sep 21, 2021
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 this pull request may close these issues.

4 participants