-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Recommend the home
crate in env::home_dir()'s deprecation
#110665
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that rust typically likes endorsing third-party crates, but maybe it's OK here. reread the discussion, yeah this is wanted.
Could you update the deprecation section in the docs to say this same thing?
@rustbot label -T-libs +A-docs
This comment has been minimized.
This comment has been minimized.
Personally i don't think that the crate being taken under cargo's wing should be construed to imply endorsement. Indeed part of the reason for doing so was that "the crate seems somewhat specific to cargo and rustup". Linking to external crates in the std docs is very unusual nowadays so I think it warrants an fcp. |
☔ The latest upstream changes (presumably #113777) made this pull request unmergeable. Please resolve the merge conflicts. |
This should be rebased onto the latest master before merge fyi |
Can I get a second opinion from @rust-lang/cargo here? My read is that the |
Agree with @joshtriplett. See also a recent issue rust-lang/cargo#12297. |
I'm afraid that means we should probably reject this PR. |
I think it is fair to say that even though the home crate is fairly rustup & cargo specific, it is being used by a lot of major packages: https://crates.io/crates/home/reverse_dependencies |
The conclusion in that issue, as I read it, is that there should be an ACP for a new version of the deprecated function. |
With the above in mind, |
I think the |
...The fact that there is any discussion at all on this PR about what crate to use suggests strongly that this case is not so open-and-shut as to recommend simply picking a crate. |
Closing this as there are concerns with this pr , and this probably needs an ACP first |
The
home
is now maintained by the cargo & rustup teams, so it seems like a no-brainer to recommend it in the standard library.See also: #71684 (comment).
Closes #71684.
@rustbot label +T-libs-api -T-lib
r? @joshtriplett
r? @Dylan-DPC