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

MSC3067: Prevent/remove legacy groups from being in the spec #3067

Closed
wants to merge 2 commits into from

Conversation

turt2live
Copy link
Member

@turt2live turt2live added kind:maintenance MSC which clarifies/updates existing spec proposal A matrix spec change proposal proposal-in-review labels Mar 18, 2021
This MSC could be perceived as blocked behind Spaces landing in Matrix, however this is not entirely
the case: given the Groups work is using a very ancient process, this MSC serves as a way to reject
the idea from entering the spec (as otherwise it would not have a chance to be considered).

Copy link

@erkinalp erkinalp Mar 18, 2021

Choose a reason for hiding this comment

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

Suggested change
## Alternatives considered
Explicitly making old-style groups rejected in both client and server sides. That will
ensure old-style groups will never occur in compliant clients and servers.

Copy link
Member Author

Choose a reason for hiding this comment

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

This suggestion needs a whole lot more words to make sense.

Choose a reason for hiding this comment

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

Clarified on the strategy on how to make them ill-formed.

proposals/3067-remove-legacy-groups.md Outdated Show resolved Hide resolved
Comment on lines +18 to +30
Groups were originally designed and deployed before there was a formal MSC review process. As such,
some clients and servers ended up implementing the feature in the stable namespace without an
appropriate spec release to back it up. This MSC proposes that the spec *not* introduce the lacking
documentation for groups. To action this, the spec would close [issue 1513](https://github.com/matrix-org/matrix-doc/issues/1513)
and all associated issues/PRs (except MSC1772, given that is Spaces), and refuse to add Groups to
the spec at a later date.

The [grammar](https://matrix.org/docs/spec/appendices#group-identifiers) for group identifiers has
already made it into the spec documents. This proposal removes that specification.

For the few non-Spaces MSCs listed on [issue 1513](https://github.com/matrix-org/matrix-doc/issues/1513),
the idea would be to propose FCP closure or mark those MSCs as abandoned/obsolete given the underlying
structure called a "group" would not be in the spec.
Copy link
Member

Choose a reason for hiding this comment

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

I'm sorry, we seem to have ended up at cross-purposes somewhat.

When we discussed the need for this MSC on MSC1772, I thought you were talking about wanting to remove stuff from the spec. As far as I can see, the only concrete proposal in that direction is the removal of group IDs from the spec, whereas the rest of it covers changes that we are not going to make to the spec. I don't feel that we need an MSC for a decision to not add stuff, and if that's what you thought I wanted, I now understand your reluctance in the MSC1772 thread.

I think all this MSC needs to say is "remove the group IDs from the appendix" (with maybe a side-note about the unspecced stuff). On the other hand, now that you've written it, I'm happy if you'd rather keep it as-is.

Or, if all this fuss is really just about the group IDs, I don't mind sticking that in MSC1772. From what you said over there it sounded like a bigger concern.

Sorry if I caused you wasted work.

Copy link
Member Author

Choose a reason for hiding this comment

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

I honestly thought there was more than just the group IDs in the spec, but it turned out to be the only thing.

If we don't need an msc to exclude groups, I'd generally prefer that spaces take up the removal of the IDs, otherwise I don't mind this MSC being the chance Groups never had to be formally rejected

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Comment on lines +25 to +26
The [grammar](https://matrix.org/docs/spec/appendices#group-identifiers) for group identifiers has
already made it into the spec documents. This proposal removes that specification.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the intention to not reuse the current group identifiers for spaces? I can't find anything in that regard, but it may have just gotten lost in the MSC1772 comments. I guess my question is, will the + sigil be reintroduced as a spaces identifier in the future, or will they just be addressed by room aliases or is there no decision/proposal made for that yet?

Choose a reason for hiding this comment

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

Spaces are rooms of rooms, you do not need another sigil to identify spaces.

Copy link
Member

Choose a reason for hiding this comment

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

Spaces are just rooms. Adding an additional identifier for each room type would get messy quickly. Plus given that they are just rooms, nothing stops Room IDs and Aliases pointing at them even if they had a dedicated additional sigil which means a lot of double handling.

Copy link
Contributor

Choose a reason for hiding this comment

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

While it is true, that adding a sigil for each room type would get messy quickly, I would appreciate knowing, what the link refers to, that I am clicking on. I don't really want to join 20 rooms, when clicking on an alias, I usually expect to join 1 room. But spaces allow joining multiple rooms by just joining the space, which may be surprising. I guess this should be solved by peeking though instead of the alias grammar, since currently nothing prevents you from joining a space by a room alias? Anyway, thank you for the explanation, that wasn't clear to me from reading the proposal!

@turt2live
Copy link
Member Author

The spaces proposal covers what this MSC does quite well, so considering obsolete.

@turt2live turt2live closed this May 1, 2021
@turt2live turt2live deleted the travis/msc/no-groups branch May 1, 2021 01:25
@turt2live turt2live added obsolete A proposal which has been overtaken by other proposals and removed proposal-in-review labels May 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:maintenance MSC which clarifies/updates existing spec obsolete A proposal which has been overtaken by other proposals proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants