Update rfcbot FCP proposal text to be less bad

[anp]

@aturon, #61 (comment):

Request: I'd like to change the text on the initial comment. Today it says:

FCP proposed with disposition to merge. Review requested from:

To make this more clear, can we say:

Team member {user} has proposed merging the RFC. The next step is review by the rest of the {team-names} team:

• {reviewer list}

Once these reviewers reach consensus, the RFC 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!

Yep! Can do.

[aturon]

So one point I should raise here: rfcbot is being used in different contexts now, not all of which are RFCs (e.g. tracking issues, code PRs). Some of its behavior probably wants to change in these various settings. For example:

• Whether "FCP" is even a thing (not true for code PRs)
• The length of FCP (6 weeks for tracking issues)
• What we'd want text as proposed in this issue to say

There are a couple ways we could deal with these differences:

• Have a different command set for different contexts/use cases
  • I'm imagining all commands are supported anywhere, but the team members know to say "rfcbot fcp stabilize" for a tracking issue, or "rfcbot pr merge" for a PR. The core behavior for all of these is the same, in terms of dashboard, checklist, etc. What changes is just the above details.
• Make the bot context-aware

I lean somewhat toward expanding the bot's vocab, and letting the humans drive these different processes by using different commands. For one thing, I think that ends up being clearer ("fcp merge" is a weird way to say "I want to merge this PR"), and I bet it ends up being easier to implement too.

[anp]

Definitely -- that makes a lot of sense. I think I also am leaning (perhaps more strongly) towards using command syntax to drive the variation. It would hopefully make the bot less brittle to process changes or use in even more contexts.

Do you see any strong advantages to making it context-sensitive to issue type and location aside from the small cognitive overhead of remembering what behavior one wants?

[aturon]

Do you see any strong advantages to making it context-sensitive to issue type and location aside from the small cognitive overhead of remembering what behavior one wants?

Nope -- the more I think about it, the more I think it should all be command syntax-driven.

[aturon]

I want to bump the priority of the text changes -- people keep getting confused with the current text. We should go ahead and change, even if adding the new commands takes longer.

[anp]

I've updated the text (with some slight modification because a) tech debt is a thing and b) it's a little more generic until I can specialize code paths for RFCs vs. PRs vs. tracking issues). Let me know if that needs further tweaking.

I've included it in the docs, but rfcbot pr merge is for now just an alias for rfcbot fcp merge.

TODO:

1. Add stabilize subcommand to fcp.
2. Add field to FcpProposal model and table to track which kind of FCP is needed (none at all for PRs, 1 week for RFCs, 6 weeks for stabilization).
3. Differentiate status comments and tagging behavior based on consensus type.

[anp]

@aturon the interim text has been used in a few places now, for example rust-lang/rfcs#1721 (comment).

I still intend to allow this to be specialized based on the chosen command, but do you think this will work for now?

[aturon]

Looks good for now, thanks!

On Tue, Oct 18, 2016 at 11:07 AM, Adam Perry notifications@github.com
wrote:

@aturon https://github.com/aturon the interim text has been used in a
few places now, for example rust-lang/rfcs#1721 (comment)
rust-lang/rfcs#1721 (comment).

I still intend to allow this to be specialized based on the chosen
command, but do you think this will work for now?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#89 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AArUr_AKvsxI8-_3bwlkBCg-2NV0L0w3ks5q1Qr5gaJpZM4KQsKb
.

[aturon]

@dikaiosune I'm labeling this P-high -- we're trying to lean more on the FCP for stabilization, and do so asynchronously, but we really need the bot to manage the timing for that to work.

As an aside, for tracking issues (B-unstable) we're now thinking the FCP length should be 3 weeks. That's because we intend to propose FCP on a rolling basis, and 3 weeks should be ample time for people to see them while still letting us fit a good number into a release cycle.

[anp]

@aturon got it.

To summarize, I'm picturing these commands:

(pr|fcp) (merge|close|postpone|stabilize) where:

• pr doesn't have any FCP period, and fcp has variable FCP lengths
• when invoked with fcp -- merge,close, postpone will all have 10 day FCPs, and stabilize will have 3 week FCPs
• when invoking pr stabilize something somewhat sensible will occur but it's not expected that people will use it

Does that make sense and accurately reflect what was above?

[aturon]

@dikaiosune

That looks great. The only thing missing: a deprecate command, akin to stabilize but for dropping features.

[anp]

@aturon so deprecate and stabilize would just do the same thing, correct? Or would they need to have separate FCP lengths?

[aturon]

They'd be the same, yes.