Clarify when API change PRs can be blocked on ACP

[oskgo]

I would like some feedback from libs/libs-api on the wording/details.

Zulip: https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/.60needs-acp.60.2C.20why.3F

r? libs-api

[BurntSushi]

I'm not a fan of the "strictly required" verbiage personally. I'd be in favor of making the language stronger ("strongly recommended" or "do not skip the ACP without a specific reason"), but I'm unsure about introducing strict requirements like this.

[BurntSushi]

Also, I don't understand the difference between "change to any user-obvious API" and "any change to API."

[tgross35]

Also, I don't understand the difference between "change to any user-obvious API" and "any change to API."

(that was my imprecise wording from Zulip attempting to distinguish interfaces/types that show up in signatures from behavior changes that are more implementation-related)

[oskgo]

It might have been a bad idea to open the PR before finishing the discussion over on Zulip. There's some more context over there.

The practical question here is when non-libs members should block progress on adding an ACP. AFAICT the answer so far has been "when they feel like it's a good idea", which somewhat contradicts the language in the dev guide.

[BurntSushi]

@oskgo Can you give a concrete example of where this wording helps? I'm honestly still having a lot of trouble following you. I've skimmed the Zulip thread.

[oskgo]

The intention is to answer "Why do I need to write an ACP?" for contributors and "Which PRs should I block on making an ACP?" for reviewers.

The right wording is going to depend on what the right answers are, and it seems like there isn't agreement on this.

[BurntSushi]

"Why do I need to write an ACP?"

I guess I felt like the wording already specifically addresses this. It's a risk mitigation strategy to avoid wasting time. It isn't a perfect system. I'd be open to try and make the wording better here.

"Which PRs should I block on making an ACP?"

I would say, "If unsure, block on ACP."

Also, my understanding is that we are supposed to have one libs-api member sign off on landing a new unstable API. Perhaps that has eroded with the existence of the ACP process. Which makes some amount of sense.

To explain more, and I'm not 100% solid on the history here, but my recollection is that "we use an FCP for API stabilization" and "one libs-api member signs off on new unstable APIs." With the advent of the ACP process, the approval of the ACP has itself become an approval of a new unstable API. And presumably, after that point, I'd guess that a new unstable API corresponding to that ACP could land without necessarily requiring the explicit approval of a libs-api member. I don't actually know if that's what's happening, but it would make sense if it was. That is, given an ACP approval, I wouldn't necessarily expect a libs-api member to need to sign off on a PR introducing the unstable API approved in the ACP.

So I think I now understand more of the reviewer problem of answering whether a PR should block on an ACP. My mental model (not necessarily speaking for the libs-api team) is that there should be at least one libs-api member that signs off on a new unstable API. The actual mechanism for that might be an ACP approval or it might be a PR approval directly that doesn't have an ACP. For a non-libs-api reviewer, I'd guess the answer is always, "since this PR is introducing a new unstable API, it needs to be approved by a libs-api member, and the process for doing that is by filing an ACP."

Apologies if this confuses things more... but it seemed like I might have connected the dots here and wanted to try to spell it out. And to be clear, the above is my recollection of the libs-api historical modus operandi, and not necessarily an immutable law that was always followed.

[oskgo]

So the stance is something like "it's never wrong to block on ACP for something that changes the API". I suppose the utility in not always requiring an ACP would be if the PR passes review before someone can get to ask for ACP, which would skip some paperwork.

I've changed the PR to just mentioning the possibility that any reviewer might ask for an ACP for an API change.

[oskgo]

I like @scottmcm's wording. Is there anything I should do to get this PR moving along?

[workingjubilee]

So the stance is something like "it's never wrong to block on ACP for something that changes the API". I suppose the utility in not always requiring an ACP would be if the PR passes review before someone can get to ask for ACP, which would skip some paperwork.

Yeah, my understanding of the intention w/r/t the "please file an ACP, but also you don't always need it" is that if something is super obvious and easy to approve, with no apparent edge-cases, conflict, or additional things sneaking in alongside it... basically, if libs-api has already said "yeah we want this" and the PR is just fulfilling that in the most obvious way... it should just land.

[BurntSushi]

Wording LGTM.