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

MSC2844: Global version number for the whole spec #2844

Merged
merged 15 commits into from
Jan 17, 2021

Conversation

turt2live
Copy link
Member

@turt2live turt2live commented Oct 30, 2020

@turt2live turt2live added kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal labels Oct 30, 2020
@turt2live turt2live changed the title [WIP] MSC2844: Global version number for the whole spec MSC2844: Global version number for the whole spec Nov 13, 2020
@turt2live turt2live marked this pull request as ready for review November 13, 2020 00:51
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

Overall I think this is a great change, which will not only encourage more frequent releases, but also simplify discussion of the Matrix spec in general. I thought your points were well thought out and I agree on the whole.

I have a few questions, suggestions and clarifications below, but otherwise this seems sane.

proposals/2844-global-versioning.md Outdated Show resolved Hide resolved
proposals/2844-global-versioning.md Show resolved Hide resolved
proposals/2844-global-versioning.md Outdated Show resolved Hide resolved
proposals/2844-global-versioning.md Show resolved Hide resolved
proposals/2844-global-versioning.md Outdated Show resolved Hide resolved
proposals/2844-global-versioning.md Outdated Show resolved Hide resolved
proposals/2844-global-versioning.md Outdated Show resolved Hide resolved
proposals/2844-global-versioning.md Outdated Show resolved Hide resolved
proposals/2844-global-versioning.md Show resolved Hide resolved
proposals/2844-global-versioning.md Outdated Show resolved Hide resolved
@turt2live
Copy link
Member Author

All the initial feedback has been addressed, and this seems to be getting supportive comments out of band. Per the MSC, this has a rudimentary implementation at https://adoring-einstein-5ea514.netlify.app/ (see "releases" dropdown in the top right) to prove that the website component works. I don't think it's practical to have an implementation proof of the remainder of the MSC - it'll have to be explored and analyzed in practice if this MSC is to be accepted.

As such,

@mscbot fcp merge

@mscbot
Copy link
Collaborator

mscbot commented Nov 22, 2020

Team member @turt2live has proposed to merge this. The next step is review by the rest of the tagged people:

Once at least 75% of reviewers approve (and there are no outstanding concerns), this 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!

See this document for information about what commands tagged team members can give me.

@mscbot mscbot added disposition-merge proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. and removed proposal-in-review labels Nov 22, 2020
@mscbot mscbot added finished-final-comment-period and removed disposition-merge final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. labels Jan 17, 2021
Copy link

@erkinalp erkinalp left a comment

Choose a reason for hiding this comment

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

Do not forget the obligatory XKCD reference about standards

Comment on lines +180 to +182
because there's no relevant specification document to link to. Even if there was, it would appear
as though we were encouraging the idea of forking a specification as a specification ourselves,
which may be confusing if not sending the wrong message entirely. Though the system proposed here

This comment was marked as off-topic.

@turt2live turt2live merged commit 5800dcb into master Jan 17, 2021
@turt2live turt2live deleted the travis/msc/global-versioning branch January 17, 2021 19:58
@turt2live turt2live added spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec and removed finished-final-comment-period labels Jan 17, 2021
@KitsuneRal KitsuneRal mentioned this pull request Feb 9, 2021
@turt2live turt2live self-assigned this Apr 6, 2021
@richvdh
Copy link
Member

richvdh commented Aug 3, 2021

Next steps on this, while I remember:

@turt2live
Copy link
Member Author

Spec PRs:

I don't believe there's anything else remaining on this MSC to cover.

@turt2live turt2live added spec-pr-in-review A proposal which has been PR'd against the spec and is in review and removed spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec labels Oct 3, 2021
turt2live added a commit that referenced this pull request Oct 12, 2021
* Update versioning specification for Matrix

As per [MSC2844](#2844)

* Mention vX versioning in /versions

* Changelog
@turt2live
Copy link
Member Author

Merged 🎉

@turt2live turt2live added merged A proposal whose PR has merged into the spec! and removed spec-pr-in-review A proposal which has been PR'd against the spec and is in review labels Oct 12, 2021
Comment on lines +203 to +204
The author does not recommend that this MSC be implemented prior to it landing due to the complexity
involved as well as the behavioural changes not being possible to implement. However, if an implementation
Copy link
Contributor

Choose a reason for hiding this comment

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

It might make sense to require an implementation even for changes like this in the future, because such large changes can always break (maybe badly written) clients in new and exciting ways: Nheko-Reborn/nheko#957

For the same reason web clients, like Firefox and Chrome, take adding even a single digit to their version number very seriously: https://hacks.mozilla.org/2022/02/version-100-in-chrome-and-firefox/

Copy link
Member Author

Choose a reason for hiding this comment

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

fwiw, the belief was that through several blog posts, notifying clients specifically, and general spec activity folks would be aware of this happening to make the required changes.

Even with an implementation, I don't believe it would have caught your specific failure mode.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I did not know that someone added validation code for that 5 years ago. I didn't even know about Matrix at the time. Having a server that exports the new version and testing clients against it sounds not that unreasonable and because of how big and varied the WWW has become, browsers actually go through the effort to do that. Since Matrix is growing too, if there is ever a version format change again (which is very unlikely), it probably makes sense to go with a similar approach and actually test if anything expects the format not to change. Testing always trumps just reading about it and guessing if it will break anything. Anyway, this is not a complaint, just something I thought I might notify people about and which might make sense to keep in mind for the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

The spec doesn't allow for experimental versions in that field, and even if it did I don't think people would be aware of a server which had the support if they weren't aware of other aspects.

Implementation and testing is always better, yes, but I don't believe it'd fix the particular problem case raised here. It's worth noting for the future though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature MSC for not-core and not-maintenance stuff merged A proposal whose PR has merged into the spec! proposal A matrix spec change proposal
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.