The new and improved MSC process #1697

anoadragon453 · 2018-10-15T00:57:11Z

We are shifting the MSC process to one that is hopefully clearer and more structured for everyone. The new process is based heavily on Rust RFCs. This PR tries to offer a complete breakdown of the process, but please do voice your concerns if something is still confusing as no doubt others will run into the same issue if not brought up now.

The process of MSCs will be aided with the use of @mscbot, our fork of Rust's @rfcbot. We hope to upstream any changes made to the rfcbot project as they are made. For core team members, documentation on using the bot is here. Expect this to change and then be put in a more presentable form as this PR evolves.

Feedback is encouraged!

specification/proposals_intro.rst

ara4n · 2018-10-15T01:02:12Z

specification/proposals_intro.rst

-  https://github.com/matrix-org/matrix-doc/tree/master/specification.  This
-  will then be reviewed and hopefully merged!  Please sign off the spec PR as
-  per the `CONTRIBUTING.rst
+- Members of the core team will review and discuss the PR in the comments and


core team & community, presumably?

i really think we should explicitly ask people to comment in threads where possible rather than have a linear-comment-stream-of-doom on the GH PR

ara4n · 2018-10-15T01:04:09Z

specification/proposals_intro.rst

+Spec PR In Review             A proposal that has been PR'd against the spec and is currently under review
+Spec PR Merged                A proposal with a sufficient working implementation and whose Spec PR has been merged!
+Postponed                     A proposal that is temporarily blocked or a feature that may not be useful currently but perhaps sometime in the future
+Closed                        A proposal which has been reviewed and deemed unsuitable for acceptance


hm, what happened to Abandoned/Obsolete/Rejected?

I've wrapped them all up into Closed as that's the model Rust takes, and I think it's fair. We could certainly continue to add an obsolete label to a closed MSC if we believe it's important to distinguish between the two.

hm. i'm not sure what value there is to losing this information, and i'm not sure 'this is what rust does' is the strongest argument. but i guess i don't care that strongly.

specification/proposals_intro.rst

ara4n · 2018-10-15T01:07:22Z

specification/proposals_intro.rst

+- Members of the core team will review and discuss the PR in the comments and
+  in relevant rooms on matrix. Discussion outside of Github should be
+  summarised in a comment on the PR.
+- At some point a member of the core team will propose a motion for a final


suspect we need to just add in a little snippet to explain what the intention of an FCP is

Further down we have:

The FCP will then begin and last for 5 days, giving anyone else some time to
speak up before it concludes.

Is this enough or should we add more?

moar please (as per other comment)

ara4n · 2018-10-15T01:08:41Z

specification/proposals_intro.rst

+The process for submitting a Matrix Spec Change (MSC) Proposal in detail is as
+follows:
+
+- Create a first-draft of your proposal using `github-flavored markdown


should probably explain why we ask people to submit their draft in GFM rather than as a PR against the spec itself, given how many people rush straight into a PR against swagger etc.

s/first-draft/first draft/

specification/proposals_intro.rst

turt2live · 2018-10-15T02:26:29Z

specification/proposals_intro.rst

-    Matrix's architectural design.
+    However, it is welcome to use an alternative room if preferred please
+    advertise it in #matrix-spec:matrix.org and link to it on the PR for
+    visibility.


This wording is a bit difficult to read (imo). How does the following sound?

If preferred, an alternative room can be created and advertised in #matrix-spec:matrix.org. Please also link to the room in your PR description.

Oh yeah, this definitely is a mixture between an old and new sentence. Thanks for pointing it out.

specification/proposals_intro.rst

turt2live · 2018-10-15T02:35:53Z

specification/proposals_intro.rst

+============================= =======================================================
+Proposal WIP                  A proposal document which is still work-in-progress but is being shared to incorporate feedback
+Proposal In Review            A proposal document which is now ready and waiting for review by the core team and community
+Proposal Final Comment Period A proposal document which has reached final comment period either for merge, closure or postponement


we should probably also clarify the labels used on all of these statuses. In particular though, I think we should have labels for people to quickly glance at what the motion is. I'm pretty sure the rfcbot does this, and we just need to add a column to this table.

turt2live · 2018-10-15T02:40:48Z

Something we might want to consider is automatically/manually opening a placeholder issue for an MSC when there's been no implementation proof for X amount of time. This placeholder would effectively be labelled as spec-pr-missing and help-wanted to indicate that the original author might not have the capacity to bring the proposal to the finish line.

Having thought of that scenario, I do also have concerns with merging the PR and the proposal dropping off our radar. Without a placeholder issue and the proposal merged, we lose visibility on it. Equally, I don't want to suggest that we leave the PR open as that is damaging in other ways. Potentially we do need a placeholder issue, or some automated process to help keep the proposal on the radar?

richvdh

great work @anoadragon453 . A few random thoughts and questions from me.

richvdh · 2018-10-15T08:28:23Z

specification/proposals_intro.rst

+The process for submitting a Matrix Spec Change (MSC) Proposal in detail is as
+follows:
+
+- Create a first-draft of your proposal using `github-flavored markdown


s/first-draft/first draft/

richvdh · 2018-10-15T08:36:57Z

specification/proposals_intro.rst

+  known as a spec PR. However in order for your spec PR to be accepted, you
+  **must** show an implementation to prove that it works well in practice. In
+  addition, any significant unforeseen changes to the original idea found
+  during this process will warrant another MSC.


could we expand on this process a bit? Should the new MSC start from scratch (in which case, what happens to the previous MSC), or propose changes from the previous MSC (so we end up with a tower of proposals)?

If the change is minor, is changing the existing MSC appropriate? Is there a process for doing so?

If the change is minor, is changing the existing MSC appropriate? Is there a process for doing so?

I believe the bot will handle it fine, but I'm not sure how that will look. A proposal successfully completing an FCP process and then getting put back on the table will seem like the FCP isn't as be-all-end-all as it should be.

I'm fine with taking the original proposal and working with that as a new MSC #, and linking to the previous as the doc states to do with similar proposals. How does that sound?

Basically my feeling is, there are two cases:

either the change is a trivial fix to something discovered in implementation; I'd prefer not to have to go all round the MSC process for that, and I think that modifying the existing proposal doc is probably fine, provided it's called out in the comments of the original PR so that anyone who commented on it gets visibility. In this case I think this does NOT count as an MSC.

or it's a more fundamental change, in which case it's a new MSC with a new doc which leaves the original alone. And yes, building it on top of an existing MSC is probably fine.

I could foresee problems with a "small change" being found after the MSC process and the fix upsetting people again who argue it's not a small change.

Presumably getting to failure mode should be relatively rare however as the proposal should detail everything the Spec PR will do very precisely?

My experience is that it's pretty common to find something that needs tweaking once you get to implementation. Here is an example. I really don't think it's appropriate to start a whole new MSC for that sort of thing; you could argue that we could have opened another FCP but even then I think it's overblown.

After some out-of-band conversation, I'm in agreeance that minor changes to the spec after acceptance is alright as long as those changes are then documented in the original proposal.

I've updated the PR to reflect this.

specification/proposals_intro.rst

richvdh · 2018-10-15T08:38:56Z

specification/proposals_intro.rst

-Rejected                    A proposal which is not going to be incorporated into Matrix
-=========================== =======================================================
+============================= =======================================================
+Proposal WIP                  A proposal document which is still work-in-progress but is being shared to incorporate feedback


proposal WIP doesn't exist on the flowchart

Renamed to "Proposal Drafting/Feedback" which coincides with the entry on the flowchart.

specification/proposals_intro.rst

richvdh · 2018-10-15T08:42:35Z

specification/proposals_intro.rst

+  addition, any significant unforeseen changes to the original idea found
+  during this process will warrant another MSC.
+
+  - Please sign off the spec PR as per the `CONTRIBUTING.rst


for reasons I can't see at a glance, this link is broken in the rendered RST.

So the sign-off is required for the spec PR but not for the initial proposal?

@KitsuneRal has a good point - we should require sign-off on the initial proposal too, to avoid folks filling up the MSC proposal list with IP of questionable provenance.

specification/proposals_intro.rst

richvdh · 2018-10-15T08:46:15Z

specification/proposals_intro.rst

+Proposal Merged               A proposal document which has passed review 
+Spec PR Missing               A proposal that has been accepted but has not currently been implemented in the spec
+Spec PR In Review             A proposal that has been PR'd against the spec and is currently under review
+Spec PR Merged                A proposal with a sufficient working implementation and whose Spec PR has been merged!


does the proposal stay in proposals forever, for historical reference?

(it seems reasonable that it should, modulo it'll be hard to tell by looking in the proposals directory what's active and what's not, but let's make it explicit somewhere)

Yes, it should, and I assume we know if it's active if it's actually merged into the repo or not.

An interesting thing to question is whether a proposal will stay in the repo if the Spec PR never appears and/or proves that for whatever reason the proposal won't work out. I'd still say yes for historical purposes.

richvdh · 2018-10-15T08:47:25Z

specification/proposals_intro.rst

+Proposal WIP                  A proposal document which is still work-in-progress but is being shared to incorporate feedback
+Proposal In Review            A proposal document which is now ready and waiting for review by the core team and community
+Proposal Final Comment Period A proposal document which has reached final comment period either for merge, closure or postponement
+Proposal Merged               A proposal document which has passed review 


Sooner or later we're going to end up with a hundred proposals in this state. Can we define some sort of process by which they get flagged as "abandoned" after a while?

oh, I think this is what @turt2live wrote at #1697 (comment).

if we don't mandate that spec PRs require implementations, we can just file a bug against synapse when the PR is merged and go from there...

richvdh · 2018-10-15T08:50:49Z

This PR fixes #1694

anoadragon453 · 2018-10-15T14:21:22Z

Having thought of that scenario, I do also have concerns with merging the PR and the proposal dropping off our radar. Without a placeholder issue and the proposal merged, we lose visibility on it. Equally, I don't want to suggest that we leave the PR open as that is damaging in other ways. Potentially we do need a placeholder issue, or some automated process to help keep the proposal on the radar?

A placeholder issue sounds like a nice idea. The bot currently doesn't do this but could certainly be ~~trained~~ reprogrammed to.

Would need to get my Rust skills up first or employ the help of another though.

specification/proposals_intro.rst

maxidorius · 2018-10-15T19:27:56Z

specification/proposals_intro.rst

    viewpoints and get consensus, but this can sometimes be time-consuming (or
    the author may be biased), in which case an impartial 'shepherd' can be
-    assigned to help guide the proposal through this process.  A shepherd is
+    assigned to help guide the proposal through this process. A shepherd is
    typically a neutral party from the core team or an experienced member of


What are the conditions to be seen as experienced?

someone who already has experience in contributing to the spec via MSCs or PRs

maxidorius · 2018-10-15T19:28:25Z

specification/proposals_intro.rst

+- Members of the core team and community will review and discuss the PR in the
+  comments and in relevant rooms on Matrix. Discussion outside of GitHub should
+  be summarised in a comment on the PR.
+- At some point a member of the core team will propose a motion for a final


"At some point" would need clarification

specification/proposals_intro.rst

maxidorius · 2018-10-15T19:32:09Z

specification/proposals_intro.rst

+- Once your proposal has been accepted and merged, it is time to submit the
+  actual change to the Specification that your proposal reasoned about. This is
+  known as a spec PR. However in order for your spec PR to be accepted, you
+  **must** show an implementation to prove that it works well in practice. A


Any kind of implementation? Can it be a stand-alone piece of code? unit tests? does it need to be integrated into existing implementations? If yes, which one are deemed "valid" to start with? Also, what defines "work well in practice"?

We deliberately leave this open ended as a suitably passable implementation could vary wildly based on the spec change it is implementing. What is deemed passable will need to be taken on a case-by-case basis.

specification/proposals_intro.rst

turt2live

Looks good from my point of view. Have suggested a few minor changes

specification/proposal_etiquette.rst

KitsuneRal

Overall I'm fine (and by the way - great text, really). A couple of non-blocking comments below.

specification/proposals_intro.rst

KitsuneRal · 2018-10-18T13:13:51Z

specification/proposals_intro.rst

+  addition, any significant unforeseen changes to the original idea found
+  during this process will warrant another MSC.
+
+  - Please sign off the spec PR as per the `CONTRIBUTING.rst


So the sign-off is required for the spec PR but not for the initial proposal?

…to anoa/msc_update

specification/proposals_intro.rst

ara4n · 2018-10-18T20:53:30Z

specification/proposals_intro.rst

+    acceptance.
+
+- Members of the Core Team and community will review and discuss the PR in the
+  comments and in relevant rooms on Matrix. Discussion outside of GitHub should


please can we have a "please use threaded comments where possible in order to separate the threads of conversation and allow them to be individually marked as resolved/unresolved"?

That's basically the opposite of what has been proposed before, in that we were going to use line comments only when it was discussing the actual text rather than the broader ideas

what was proposed before was wrong, then ;)

trying to follow multiple streams of discussion in github comments is a nightmare.

The issues raised were that it is very easy for the conversations to get lost, and hard to follow updates unless you're actively participating in all the discussions. Amongst others

Either way, I'm not sure we want to block this MSC process doc on deciding whether we should change it.

yup, what was proposed before was wrong. empirically, the new GH process seems to be working well when people comment in threads (which can be individually tracked and resolved/unresolved). It becomes a complete trainwreck when there's just one massive linear thread which jumps around all over the place without any way to track resolution of sidebars or follow the thread. I think specifying how we review the MSCs is pretty important, and i'd prefer that we got on the same page.

(imagine how annoying it'd be if this discussion was interleaved with all the other discussions on this MSC, in the main comment thread...)

I can understand that people find it better, but I don't think its a trivial change that should be put in without some discussion with the rest of the spec team tbh.

ara4n · 2018-10-18T21:02:43Z

specification/proposals_intro.rst

+follows:
+
+- Create a first draft of your proposal using `GitHub-flavored markdown
+  <https://help.github.com/articles/basic-writing-and-formatting-syntax/>`_


A historical note might be good here to acknowledge that we used google docs in the past but wanted to switch to a more FOSS-friendly system with better versioning, and to acknowledge that there are grandfathered MSCs hanging around.

We have one here. Should it be moved?

https://github.com/matrix-org/matrix-doc/pull/1697/files#diff-587ffef8f8d50dd2caa2a1c06a5697b1R242

ara4n

i have a few minor requests remaining (most notably countermanding @richvdh's request to move the philosophical content off centre-stage - sorry :/), but otherwise looks excellent to me. can't wait to use it!

richvdh

lgtm

ara4n · 2018-10-20T22:48:27Z

lgtm, thanks :)

richvdh · 2018-10-23T14:46:52Z

specification/proposals_intro.rst

+Proposal Drafting and Feedback   N/A                            A proposal document which is still work-in-progress but is being shared to incorporate feedback
+Proposal In Review               proposal-in-review             A proposal document which is now ready and waiting for review by the Core Team and community
+Proposal Final Comment Period    proposal-final-comment-period  A proposal document which has reached final comment period either for merge, closure or postponement
+Proposal Merged/Spec PR Missing  proposal-passed-review         A proposal document which has passed review. Waiting for a PR against the Spec


is the label for this not "spec-pr-missing" ?

Explained in the other comment.

richvdh · 2018-10-23T14:47:07Z

specification/proposals_intro.rst

+Proposal In Review               proposal-in-review             A proposal document which is now ready and waiting for review by the Core Team and community
+Proposal Final Comment Period    proposal-final-comment-period  A proposal document which has reached final comment period either for merge, closure or postponement
+Proposal Merged/Spec PR Missing  proposal-passed-review         A proposal document which has passed review. Waiting for a PR against the Spec
+Spec PR In Review                spec-pr                        A proposal that has been PR'd against the spec and is currently under review


spec-pr-in-review? on the proposal pr or the spec pr?

Good point. I was thinking this would be a lifetime label but that doesn't really make sense once it's merged. Also this would be on the proposal. Added some clarification that all labels should be on the proposal PR.

ara4n · 2018-10-23T18:02:52Z

lgtm, let's ship it!

AnymeanSomath

[d487c09 ]

AnymeanSomath · 2024-10-01T20:07:11Z

Move to the new and improved MSC process

a3144e6

ara4n reviewed Oct 15, 2018

View reviewed changes

specification/proposals_intro.rst Show resolved Hide resolved

ara4n reviewed Oct 15, 2018

View reviewed changes

specification/proposals_intro.rst Outdated Show resolved Hide resolved

ara4n reviewed Oct 15, 2018

View reviewed changes

specification/proposals_intro.rst Outdated Show resolved Hide resolved

ara4n reviewed Oct 15, 2018

View reviewed changes

turt2live reviewed Oct 15, 2018

View reviewed changes

richvdh requested changes Oct 15, 2018

View reviewed changes

anoadragon453 added 9 commits October 15, 2018 15:29

Add proposal template

764c63f

Note about earlier MSC process

d2e827e

Fix capatilization

80d1d4a

first-draft -> first draft

9432ed7

MSC ALL the changes

67146e4

Put it in the proposals dir!

f769084

Fix wording, fix earlier capatilization

0e9ad9e

Less complicated wording

f1d71bd

Require link to implementation

5ae3b50

anoadragon453 added 4 commits October 15, 2018 16:30

Proposal WIP -> drafting/feedback gathering

11dbd5b

Proposal Created and In Review is a single phase

873641e

proposal passed = spec pr missing

b3f90f5

Attempt to fix contributing link

6040913

maxidorius reviewed Oct 15, 2018

View reviewed changes

anoadragon453 added 3 commits October 15, 2018 23:37

No need to redefine MSC

fb4c50e

your proposal -> the proposal

49d017b

Move proposal etiquette to a separate document.

889b468

anoadragon453 added 3 commits October 17, 2018 14:49

Gather feedback

1e939c5

Clarify that MSCs aren't just issues anymore.

45c935d

Clarify how to get a shepherd (just ask)

d487c09

turt2live approved these changes Oct 17, 2018

View reviewed changes

specification/proposal_etiquette.rst Outdated Show resolved Hide resolved

specification/proposal_etiquette.rst Outdated Show resolved Hide resolved

turt2live added 2 commits October 17, 2018 18:20

reduce abbreviations

9625f11

Reduce more abbreviations!

5a2d28e

KitsuneRal approved these changes Oct 18, 2018

View reviewed changes

anoadragon453 added 2 commits October 18, 2018 15:52

be begin -> begin

7bff4d2

Merge branch 'anoa/msc_update' of github.com:matrix-org/matrix-doc in…

ea21e31

…to anoa/msc_update

ara4n reviewed Oct 18, 2018

View reviewed changes

specification/proposals_intro.rst Outdated Show resolved Hide resolved

ara4n reviewed Oct 18, 2018

View reviewed changes

specification/proposals_intro.rst Outdated Show resolved Hide resolved

ara4n reviewed Oct 18, 2018

View reviewed changes

ara4n requested changes Oct 18, 2018

View reviewed changes

richvdh approved these changes Oct 18, 2018

View reviewed changes

anoadragon453 added 2 commits October 20, 2018 23:35

Etiquette -> guiding principles

f00be6b

Clarification on FCPs

0afb122

anoadragon453 added 2 commits October 21, 2018 00:52

DCO required for proposal and spec PR

4b858a7

Remove etiquette page from targets

2b259c6

richvdh reviewed Oct 23, 2018

View reviewed changes

Update and clarify proposal labels

f8ffa79

anoadragon453 merged commit f288fac into master Oct 23, 2018

anoadragon453 deleted the anoa/msc_update branch October 23, 2018 21:19

ara4n mentioned this pull request Oct 30, 2018

clarify the MSC process #1694

Closed

AnymeanSomath approved these changes Oct 1, 2024

View reviewed changes

matrix-org locked as resolved and limited conversation to collaborators Oct 1, 2024

The new and improved MSC process #1697

The new and improved MSC process #1697

Conversation

anoadragon453 commented Oct 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turt2live commented Oct 15, 2018

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richvdh commented Oct 15, 2018

anoadragon453 commented Oct 15, 2018

maxidorius Oct 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turt2live left a comment

Choose a reason for hiding this comment

KitsuneRal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ara4n left a comment

Choose a reason for hiding this comment

richvdh left a comment

Choose a reason for hiding this comment

ara4n commented Oct 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ara4n commented Oct 23, 2018

AnymeanSomath left a comment

Choose a reason for hiding this comment

AnymeanSomath commented Oct 1, 2024

maxidorius Oct 15, 2018 •

edited

Loading