Proposal for speeding up review of simple spec changes

[ara4n]

Currently the spec proposal process implies that any change to the spec should go through a written proposal doc (which is then reviewed and gathers consensus) and then a spec PR (which is reviewed and merged). The intention of this is to avoid contributors wasting their time by writing proposing spec PRs on a subject which has not yet been adequately designed or discussed.

However, I'd like to propose that for changes which are particularly simple (such as those which can be entirely described and justified in a 10 line GH issue) and uncontroversial (e.g. fixing spec omissions by documenting longstanding existing behaviour, or adding trivial features) it may be acceptable to skip the written proposal stage and instead gather review & consensus on the spec PR. Of course, a spec PR reviewer should push back and request a proposal if one is actually needed; we are not trying to encourage people to shortcut the consensus process but to minimise bureaucracy for trivial changes. Given how far behind we are on reviewing spec proposals, it would be a disaster if our process unnecessarily slowed things down further.

For example: this issue itself (if it was a PR) is a great example of a spec change proposal which is so simple that it doesn't justify a formal googledoc proposal, and creating one would just waste time copy-pasting the above text between Google & GH. If during review there's coherent feedback that this is more complex and controversial than it originally seems, then we'd of course fully flesh it out as a doc proposal.

[maxidorius]

if it's something that can be fully described and justified in a paragraph of text and doesn't make any controversial assumptions or design choices then i'd argue that it's trivial (Source)

such as those which can be entirely described and justified in a 10 line GH issue

You wrote 3 paragraphs and 16 GH issue lines, so it doesn't qualify right?

[non-Jedi]

I agree with this change wholeheartedly, but I want to make sure this isn't used to backdoor things like #1290 and #1292 that represent pieces of new work from a broader whole of enabling GDPR compliance. There needs to be a valid spec proposal for all of the GDPR work and not just random bits of it trickling in as "spec-omission"s.

[turt2live]

I believe I mentioned it somewhere in #matrix-spec, but I'd hope that people would complain if they disagree about things being spec-omissions (as you've done). In the case of GDPR, I agree and would say they have been miscategorized.

[richvdh]

To be honest, I hadn't considered "spec-omission" to necessarily mean "should end up in the spec without further discussion". It's just a label to say "synapse is doing this thing which isn't in the spec".

Maybe we need more labels...

[turt2live]

This has been open for a week without further comment, and I believe it has been hashed out in the various chats a couple times over.

Can it be stamped with a formal goahead and brought into PR form or is there feedback I'm not aware of which prevents this?

[ara4n]

nobody has objected apart from Max, who seemed worried about there not being enough opportunity for gathering review or consensus at the PR stage, or that it was somehow undermining the existing process by altering the flow relatively early on.

my position is that there is just as much room for review and comments at the PR stage, and if there’s legitimate demand for a Proposal we would always go and ask for one anyway. Meanwhile, our aim here is to go fast and have a process which doesn’t slow us down, so iterating on the process to make it go faster is vital.

If I could get some thumbs up/down on this comment from @turt2live @KitsuneRal @mujx @richvdh @dbkr @uhoreg @erikjohnston @anoadragon453 then we can proceed.

[maxidorius]

At the time of my first objection, I gave links to several PRs that were made without proposals (but not yet merged) and links to PRs that were merged without spec proposals.

There was an agreement that something didn't work out as expected and that skipping proposal on some of those was a mistake.

Now, today, we get the same kind of issue for #1329, which totally bypassed the process, was only open for 3h, then triggered an issue #1331 that there was a spec bug introduced by #1329 that was just about to be merged and directly affected #1330 which was modified, and triggered a big talk as being a controversial point.

#1330 was also initially a spec PR without a proposal, but then had to be turned into a proposal also because it wasn't as easy as it sounded.

I think at this point, there has been several instances of:

• Misjudgement of complexity, both by the person doing the spec PR and those reviewing it
• Abuse of the system
• Side effects on other proposals
• Impossibility for the community to give a thoughtful feedback, as everything needs to be rushed and move fast, at the cost of actually improving the spec

I am strongly against this proposal as there are evidences that it's not working. Instead, people should try to stick to the process for a little while and then see how it can be improved, instead of directly trying to bypass the process.

[anoadragon453]

I think there's fault on both sides here. One from merging a spec change without a proposal before this issue was resolved, and two from discussions on spec changes moving too slowly leading to people involved becoming frustrated and wanting faster constructive feedback.

Perhaps judging a spec change on its minuteness isn't the right way to go. I believe that fixing a typo or formatting in the spec shouldn't have to go through a formal review process, but rather a simple github PR process will do fine. Something as simple as adding a new key to an endpoint could potentially have far-reaching effects, even though it is structurally a very small change.

I propose to come up with a non-ambiguous method of determining whether a change requires the formal review process. The typo/formatting versus anything else feels enough for me at the moment.

Additionally, some more structure around the review process itself may help prevent it from becoming a messy situation. We currently have the stages of review, though at the moment they feel more like a categorization aide. It's not entirely clear what a proposal author must do after reaching a certain stage, and I feel that that ambiguity leads to confusion and frustration.

Whether a time-based limit be added to each stage, or rules where a set % majority of people must approve it, or anything else I'm not sure. I'm willing to look at other community and see how they handle this however.

As an additional note, some have expressed frustration in setting up a proposal for a rather small change. A proposal template with some basic instructions (much akin to a GitHub issue template), may make the task feel less cumbersome.

[maxidorius]

@anoadragon453 I have no problem with a direct PR to change formatting or typo, as long as there is no functional change. I never ever considered those to be part of the spec proposal process, and PRs that did that were not included in my list. My feedback is strictly on those who provide a functional change, or attempt to "clarify" things, as they implicate an effective or possible behaviour change.

[turt2live]

It's worth noting that every attempt I've made at trying to follow the proposal process has ended in dead air, some with so much dead air I forgot I even wrote them. I've also tried to go the route of proving this proposal in practice, however in each case it was found that several people didn't want to even try it, which is sad and infuriating.

I'm on board with whatever helps make the spec process approachable, engaging, collaborative, constructive, and efficient. As it stands, the community clearly wants the spec to move forward, however road blocks keep being thrown up that prevent that. The majority of proposals I've seen have been met with hostility and complaints rather than constructiveness and collaboration.

This proposal has aspects which make the process more approachable for people who are new to contributing to the spec. By not having to do a ton of paperwork they are able to see their dreams realized faster, encouraging future, more complicated, proposals. Likewise, by documenting the current behaviour the spec is complete enough to be iterated on. As you yourself have said @maxidor, without the current behaviour being documented it is hard to review something. I do agree that PRs which aim to fill in spec omissions should be constrained exclusively to current behaviour, nothing more and nothing less. Anything more would require a proposal for sure.

Clearly the current process isn't efficient. That obviously needs work, and I strongly believe this proposal will help with that, for obvious reasons.

[anoadragon453]

Worth noting how the Rust team does things.

[ara4n]

@turt2live I'm trying to do everything I can to add bandwidth to the process of reviewing and incorporating MSCs.

@anoadragon453 I think my original proposal has got slightly lost here. I'm not suggesting that we skip formal review for small changes (as one might for typographic/cosmetic ones). I'm just suggesting that we skip breaking out Google Docs if the justification for a small change (like this one) can be fully expressed in a GH description and we believe the change is uncontroversial. The change still needs to go through as much review as ever, which culminates in a bunch of thumbs up (or down) from the core team as per #1308 (comment). But it just stays in a single place (a GH issue like this one) rather than being split and potentially desynced.

Now, I totally agree that we should put some minimum & maximum time limits on gathering feedback & completing review, in order to give everyone a chance to give feedback, and to avoid MSC or PRs getting stuck forever. This is precisely the sort of detail I'd like to flesh out as part of #1318 - i.e. how the spec team gets consensus/consent/agreement/acceptance on changes, and how rapidly that happens. Given #1318 is literally concerning how/what to borrow from Rust, I suggest the consensus discussion goes there.

If folks can provide reactions to the original proposal on the feedback gathering comment it'd be appreciated. (I'm trying to do this with feedback gathering comments rather than the actual issue so that it reflects people's opinions after an informed debate rather than before it).

One other thing:

The majority of proposals I've seen have been met with hostility and complaints rather than constructiveness and collaboration.

This +100000000. The damage being done by pedantically and aggressively challenging each and every proposal, however minor, is massively demoralising and slowing down the whole process (and is even more frustrating if the process then gets the blame). How on earth are we going to work through the backlog let alone improve the spec if any small point ends up sucking up multiple man-days of time? I'm currently at the point where bystanders are non-ironically asking me whether these pushbacks are a deliberate attempt to sabotage progress on the spec. This is one of the reasons we have been hesitant to prematurely deploy an open governance process. If you find yourself repeatedly throwing up roadblocks, please take a long hard look at whether your actions are actually for the greater benefit of the project or not.

[anoadragon453]

I see, thank you for clarifying. I've given my vote.

[KitsuneRal]

Just to leave my feedback to add on top of written before: I'd really appreciate if the number of places I have to look at and links to follow has been brought to the minimum. Therefore I'd really prefer to have the entire proposal in a GH issue instead of having an issue and a one- or two-page GoogleDoc with a couple of comments in it. I'm certainly for distinguishing between documenting the current behaviour (which should happen is fast as possible - sometimes not necessarily in the spec though) and proposing some new behaviour - these better be discussed in separate GH issues. Otherwise please let's not become a subsidiary of ISO before we have its level of responsibility (with all due respect, we're not even remotely there yet).

[maxidorius]

Disclaimer: Also posting this on behalf of other community members

---

So today this happened: #1383

• Related code has been made available to prod systems before the issue was even created [1] [2]
• Reviewed by the whole spec team
• Skipped involving relevant projects in the ecosystem, making the actual review rate at best 50% (synapse, dendrite, construct, mxhsd) and at worst 33% (synapse, dendrite, construct, mxhsd, ruma, plasma)
• Skipped community review overall, possibly sending the message the community is not important
• Straight to passed review (!)
• And yet, doesn't actually achieve anything as naively bypassed or state reseted in various ways.
• Took the community 30 sec to see how flawed it was and, when down to protecting against actual attacks, useless. [1] [2] [3]

And the best part of it all might be those quotes from the doc for a security fix, which is... interesting:

... room admins will need to keep the server_acl event updated, and block any servers not updated to handle it.

This is meant as a quick and temporary solution until there is a new version of the room protocol that natively supports server bans.

If we had to pick one proposal that could have really used the review process, it was this one.

[ara4n]

This issue is about skipping the proposal process for trivial changes; whilst #1383 was handled as a security issue (and the proposal process wasn't skipped, but done under embargo), so I'm going to move this comment over to #1383 and respond to it there....

[dbkr]

Yep, agreed in principle this should definitely be a thing, and as far as I'm concerned it's a bug if it isn't already formalised. If correcting a spelling mistake requires writing a google doc then this is a world gone topsy-turvy.

There's a lot of debate here on the rules for what changes can use this system so let's not try to define it like this: I'd say if you want to start by writing a formal PR in markdown, go ahead, but as you say, if a reviewer disagrees that it's as simple as you thought it was, it goes to a proposal doc and you might have to rewrite all your markdown once it comes out the other side.

I assume the intention here is that this becomes a modification to https://github.com/matrix-org/matrix-doc/blob/master/specification/proposals_intro.rst ?

[ara4n]

I'm trying to clear up the confusion that surrounds the MSC process and actually resolve the meta-MSCs about it.

The conclusion on this one in the end is that a) it's obsoleted by #1426, and b) there was broad agreement that pre-r0 we can apply some discretion on moving fast when documenting spec omissions, but once we actually cut a formal stable release, any functional change to the spec (however minor) should need to go through MSC. Meanwhile, pre-r0 we'll also do MSCs for major changes (e.g. the remaining S2S stuff, as we did for state res v2).