-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
docs: Declare support level for each crate in our Charter / docs #14600
Conversation
Team member @epage has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
This is to bring us into conformance with the [Rust crate ownership policy](https://forge.rust-lang.org/policies/crate-ownership.html). Items of note - `cargo-credential-1password` is declared as Experimental as it is intended for the community but I was unsure if we wanted to commit to full support for it. In my mind, the ideal thing to do would be to expatriate this to 1password. - `home` is declared as Internal despite its wide use within the ecosystem. - `cargo-credential` is declared as Intentional as its an API intended for the wider ecosystem and I didn't see a reason to declare it experimental. - `cargo-platform`, `cargo-util-schemas`, and `crates-io` are declared as Intentional as they are both used internally and intended for others to use for logic that integrates with cargo/registries. I wondered about these being Experimental or Internal instead.
70575a0
to
2662d63
Compare
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.
Thanks for putting this together, it looks great!
crates/rustfix/README.md
Outdated
@@ -9,6 +9,10 @@ This is a low-level library. You pass it the JSON output from `rustc`, and you c | |||
|
|||
If you are looking for the [`cargo fix`] implementation, the core of it is located in [`cargo::ops::fix`]. | |||
|
|||
> This crate is maintained by the Cargo team, primarily for use by Cargo |
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.
Hm, this one is a bit tricky. It is used by compiletest
and ui_test
. I wonder about making this a little more broad, such as "Cargo and the Rust compiler test suite" or something like that?
> This crate is maintained by the Cargo team for use by the wider | ||
> ecosystem. This crate follows semver compatibility for its APIs. |
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'm on the fence here. My intention when I created it was to be more "internal". I was thinking that if others found it useful, we could expand that to a more "experimental" status. I didn't know if anyone would actually have a use for it, and it wasn't really something I wanted to put effort into.
Also, as evident by the lack of a README, this isn't really up to my standards for an intentional artifact that we intend to support.
It looks like it has a few dependents now, though: https://crates.io/crates/cargo-platform/reverse_dependencies.
I'm fine with elevating this to be "intentional" if ya'll agree. It has required almost no maintenance effort, and I don't expect us to delete it, and we already have semver checks. I just wanted to make clear what I was originally thinking.
(We probably should eventually add a real README, though 🤣)
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.
Thanks for the clarification!
I think its reasonable for the Rust Project to provide an API for this micro-DSL. My main concern is what shape any of that should take and thats where I feel like the commitment of "Intentional" is strong. Granted, the definition of "Intentional" is strong enough I'm hesitant to mark anything intentional but anything else would also be too weak / scare people away.
This is probably should say something. I'm personally fine with putting it in the root README. Can you say more about your concern about the messaging? Is it related to the README having different audiences? |
I think thats a way of summing it up. If someone goes to the Cargo repo to learn about Cargo and sees a big banner saying that Cargo is experimental or internal, that will send the wrong message. However, we do want to send a message like that to anyone who builds the binary directly or tries to depend on it as a library. |
I think it should be fine to include the notice in the root README with the right wording to make it clear what it means. The README already has some low-level documentation for building cargo, which is for a very limited audience. |
I've done a pass at wording for |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
@@ -9,6 +9,13 @@ Cargo downloads your Rust project’s dependencies and compiles your project. | |||
[The Cargo Book]: https://doc.rust-lang.org/cargo/ | |||
[Cargo Contributor Guide]: https://rust-lang.github.io/cargo/contrib/ | |||
|
|||
> The Cargo binary distributed through with Rust is maintained by the Cargo |
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.
This is already mentioned in the "Installing Cargo" section.
Should we move "For all other uses of this crate..." to under "Compiling from Source"? Something like
## Compiling from Source
While [the `cargo` crate](https://crates.io/crates/cargo) is avaiable via crates.io,
compiling it from source is not intended for external use,
regardless for using either as a binary or library.
This crate may make major changes to its APIs.
### Requirements
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.
That section heavily implies "building the binary from source" and doesn't really cover the "depending on it as a library" case.
I feel like a lot of this should be re-worked, mostly by moving in to the contributor guide. I recommend we remove the install instructions and defer re-working the compilation instructions.
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.
Since Eric's concern has been addressed. I am going to merge this.
If there are other concerns, we can address later.
@bors r+ |
☀️ Test successful - checks-actions |
Update cargo 17 commits in 80d82ca22abbee5fb7b51fa1abeb1ae34e99e88a..ad074abe3a18ce8444c06f962ceecfd056acfc73 2024-09-27 17:56:01 +0000 to 2024-10-04 18:18:15 +0000 - test: Remove the last of our custom json assertions (rust-lang/cargo#14576) - docs(ref): Expand on MSRV (rust-lang/cargo#14636) - docs: Minor re-grouping of pages (rust-lang/cargo#14620) - docs(ref): Highleft whats left for msrv-policy (rust-lang/cargo#14638) - Fix `cargo:version_number` - has only one `:` (rust-lang/cargo#14637) - docs: Declare support level for each crate in our Charter / docs (rust-lang/cargo#14600) - chore(deps): update tar to 0.4.42 (rust-lang/cargo#14632) - docs(charter): Declare new Intentional Artifacts as 'small' changes (rust-lang/cargo#14599) - fix: Remove implicit feature removal (rust-lang/cargo#14630) - docs(config): make `--config <PATH>` more prominent (rust-lang/cargo#14631) - chore(deps): update rust crate unicode-width to 0.2.0 (rust-lang/cargo#14624) - chore(deps): update embarkstudios/cargo-deny-action action to v2 (rust-lang/cargo#14628) - docs(ref): Clean up language for `package.rust-version` (rust-lang/cargo#14619) - docs: clarify `target.'cfg(...)'` doesnt respect cfg from build script (rust-lang/cargo#14312) - test: relax compiler panic assertions (rust-lang/cargo#14618) - refactor(compiler): zero-copy deserialization when possible (rust-lang/cargo#14608) - test: add support for features in the sat resolver (rust-lang/cargo#14583)
What does this PR try to resolve?
This is to bring us into conformance with the Rust crate ownership policy.
Items of note
cargo
does not have a status specified on it.cargo install cargo
shouldn't be done andcargo add cargo
is "internal" but putting in our README that Cargo is "internal" feels weird from a messaging perspective.cargo-credential-1password
is declared as Experimental as it is intended for the community but I was unsure if we wanted to commit to full support for it. In my mind, the ideal thing to do would be to expatriate this to 1password.home
is declared as Internal despite its wide use within the ecosystem.cargo-credential
is declared as Intentional as its an API intended for the wider ecosystem and I didn't see a reason to declare it experimental.cargo-platform
,cargo-util-schemas
, andcrates-io
are declared as Intentional as they are both used internally and intended for others to use for logic that integrates with cargo/registries. I wondered about these being Experimental or Internal instead.How should we test and review this PR?
I was mixed on what to do with unpublished crates (and didn't fully check what all is unpublished).
I ended up only skipping xtasks.
Additional information