-
Notifications
You must be signed in to change notification settings - Fork 378
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
MSC3825: Obvious relation fallback location #3825
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Nicolas Werner <nicolas.werner@hotmail.de>
Signed-off-by: Nicolas Werner <nicolas.werner@hotmail.de>
- Thread (as of writing) are not part of a released spec version. They are | ||
marked as beta and users expect some things to not work optimally. As such | ||
minor breakage should be acceptable. They will become part of the spec in | ||
presumably end of Q3 2022. If we want to fix it, now is probably a better | ||
time than later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Beta" is not a concept at the spec level: threads are stable enough for usage. Minor breakage is acceptable, however I question that this MSC is minor in nature: while its line diff is small, it affects a very significant feature of threads themselves.
I feel like this MSC might, unfortunately, be subject to backwards compatibility clauses of the spec.
also, mentioning timelines doesn't really help the argument here - would just remove them. When threads get released is irrelevant to how much of a breaking change something is ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beta are not a spec level concept, but it is a clear marking in clients that currently support threads. Very few clients support them and they still have significant usability "bugs" currently, like notifications/read receipts in threads being wonky. Users accept that behaviour already, so presumably they would also accept other breakage if there is a reasonable transition. This change can be implemented in a manner that is only noticeable to people scrolling back 3 months of history in their chat history. While that does happen, it is a somewhat rare nonoccurence to have opted into threads and scroll back that far in the history in a room with a thread.
The timelines are there so that people who are not part of the SCT can visualize them. While those are of course subject to change as always, I think knowing that threads won't be in v1.3 of the spec is an important bit of knowledge when evaluating this MSC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @deepbluev7 here on the rarity argument. This is IMO a fairly large semantic wart. Since the fallout of a fix would be rather small, it makes sense to fix it sooner rather than have to add new fields for new types of fallbacks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from threads now being in the specification, I still strongly believe that backwards compatibility is important here.
Also, having written the spec on threads, I don't really believe the argument that the location of is_falling_back
is unclear.
Further, we don't really try to optimize for future unknown cases. In fact, for this particular case we'd be more interested in an overhaul of relations than yet another fallback mechanism or relation type.
Rendered
Signed-off-by: Nicolas Werner nicolas.werner@hotmail.de