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

Rework the feature lifecycle #55

Merged
merged 1 commit into from
Jul 4, 2023

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented May 19, 2023

This PR reworks the workflow for introducing new APIs to the standard library. This is the result of extensive discussion in the 2023 libs meetup.

Major changes include:

  • The role of ACPs is now much smaller: their role is to decide whether a feature is suitable/desirable for inclusion in the standard library, rather than to iterate on the exact API design.
  • API design exploration happens in community discussions, outside the purview of the libs team. A summary of this discussion should be included when the feature is submitted as a PR to justify the choice of API.
  • ACPs are regularly reviewed by the library team in weekly meetings.

cc @rust-lang/libs-api

src/feature-lifecycle/summary.md Outdated Show resolved Hide resolved
src/feature-lifecycle/summary.md Outdated Show resolved Hide resolved
Copy link
Member

@the8472 the8472 left a comment

Choose a reason for hiding this comment

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

A general note on risk/effort/wait time tradeoff should be mentioned. E.g. that one can start with a PR first and do the ACP later but that faces a higher risk of wasting effort. But on the other hand there's always a possibility that things will get rejected during later parts of the process up to the FCP.

To an experienced contributor this is all obvious but it might not be to a newcomer.

src/feature-lifecycle/summary.md Outdated Show resolved Hide resolved
src/feature-lifecycle/summary.md Outdated Show resolved Hide resolved

## API design exploration

Once a feature is deemed suitable for inclusion in the standard library, the exact design should be iterated on to find the best way to express it as a Rust API. This iteration should happen in community forums such as [Rust internals](https://internals.rust-lang.org/) where all members of the community can comment and propose improvements.

Choose a reason for hiding this comment

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

such as Rust internals

"Such as" implies that there might be other appropriate places. Are there? If so, which ones?

who will evaluate the proposal and accept it if they are optimistic that the proposal will
be merged and pass its eventual FCP.

You can create an ACP in the `rust-lang/libs-team` repo using [this issue template](https://github.com/rust-lang/libs-team/issues/new?assignees=&labels=api-change-proposal%2C+T-libs-api&template=api-change-proposal.md&title=%28My+API+Change+Proposal%29). This should include a sketch of the proposed API, but does not have to be the final design that will be implemented.

Choose a reason for hiding this comment

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

It looks like this issue template links to the deleted api-change-proposals.md page. The template should probably be changed before merging this.

Copy link
Member

Choose a reason for hiding this comment

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

I still see it there?
image

Copy link

@Victor-N-Suadicani Victor-N-Suadicani May 28, 2023

Choose a reason for hiding this comment

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

I think you misunderstood :) I mean in the template you show, there is a link to https://std-dev-guide.rust-lang.org/feature-lifecycle/api-change-proposals.html which this PR seems to delete. So that link would be dead after this merges.

Keep the following points in mind during the discussion:
- Resist the temptation to make the API overly general since this tends to increase complexity and lowers the chance of it being accepted by the library API team. Instead, try to stay focused on the problem that needs solving.
- An alternative that should *always* be considered is simply adding this feature via a third party crate. This is even possible when adding new methods to standard library types by using extension traits.
- In the case of "convenience" functions which are simply shorthands for something that is already possible with existing APIs, the cost of extending the standard library surface should be weighed against the ergonomic impact of the new functions.

Choose a reason for hiding this comment

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

the cost of extending the standard library surface

Could this be more specific about what this cost is? The increase in documentation length? The maintenance cost (due to the stability guarantees, I imagine many APIs in std will rarely if ever need to change)?

I mention this because I think improving ergonomics is a very valid reason for introducing new APIs and shouldn't lightly be dismissed.

src/feature-lifecycle/summary.md Outdated Show resolved Hide resolved
@m-ou-se
Copy link
Member

m-ou-se commented Jun 26, 2023

In #56 I removed the old page on ACPs. Sorry for the merge conflict. I guess in the new layout this should go in chapter 2.

@jyn514
Copy link
Member

jyn514 commented Jul 1, 2023

I am planning to move https://rustc-dev-guide.rust-lang.org/stability.html from the rustc guide to the std guide soon, but I'll wait until this is merged to avoid conflicts.

@Amanieu
Copy link
Member Author

Amanieu commented Jul 3, 2023

I've rebased and address all the feedback.

@m-ou-se m-ou-se merged commit 64f2065 into rust-lang:master Jul 4, 2023
github-actions bot pushed a commit that referenced this pull request Jul 4, 2023
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.

7 participants