Better description of the mainline of a PL event. #1107

DMRobertson · 2022-06-01T14:52:36Z

Resolves #1106.

Preview: https://pr1107--matrix-spec-previews.netlify.app

``iteratively descending through the `m.room.power_levels` events in the `auth_events`-starting at *e*.`` was also unclear to me. Unfortunately this is now more symbol-dense and I'm not sure if it's a net improvement.

DMRobertson · 2022-06-01T14:59:06Z

I'm not as convinced by e2d8cfc. I'll leave the reviewer to judge.

dkasak · 2022-06-02T14:51:32Z

content/rooms/fragments/v2-state-res.md

-descending through the `m.room.power_levels` events in the `auth_events`
-starting at *e*. If no mainline event is encountered when iteratively
-descending through the `m.room.power_levels` events, then the closest
+Given another event *e* = *e<sub>0</sub>* we can compute a similar list of


The old definition didn't seem to allow for e (e_0) itself to be the closest mainline event to e, but the new one seems to allow it. Is this correct?

Also, is e meant to be an arbitrary event here or another m.room.power_levels event?

Not sure, I'll consider this more carefully.

The old definition didn't seem to allow for e (e_0) itself to be the closest mainline event to e, but the new one seems to allow it. Is this correct?

I think there is some leeway here. What matters is that part (1) of the mainline ordering is unchanged (we don't use this notion of closest mainline event anywhere else AFAICS).

Also, is e meant to be an arbitrary event here or another m.room.power_levels event?

I think e is arbitrary, including the possibility that e is another m.room.power_levels event.

I think there is some leeway here. What matters is that part (1) of the mainline ordering is unchanged (we don't use this notion of closest mainline event anywhere else AFAICS).

The idea I had was: what if e is a m.room.power_levels event itself and it has no m.room.power_levels in its auth_events? Then e would be the closest mainline event to e according to the new definition, but not according to the old one.

In any case, looks to me like you fixed the concern in the new version 👍🏻

Then e would be the closest mainline event to e according to the new definition, but not according to the old one.

Agreed. And I think that change would be a mistake. My rough sense of this is that we're trying to see where e fits into the history of power level events, so that "older" events are ordered first.

content/rooms/fragments/v2-state-res.md

Co-authored-by: Denis Kasak <[email protected]>

This has undergone a fairly large rework, so needs careful inspection.

looks better in Firefox. I am sad to be writing a poor man's TeX though.

DMRobertson · 2022-06-06T13:14:31Z

I have rewritten things more dramatically; this needs careful interrogation,

https://pr1107--matrix-spec-previews.netlify.app/rooms/v9/#definitions for the rendered view.

DMRobertson · 2022-06-06T13:14:55Z

I can't request reviewers, but I'd also appreciate @erikjohnston here

dkasak

Left some minor suggestions, but overall this reads much more clearly to me and is a definite improvement over the current situation. I also believe it is a correct translation of the original text, but I'd love Erik to see it too.

dkasak · 2022-06-08T13:54:24Z

content/rooms/fragments/v2-state-res.md

+event in its `auth_events`.
+The *mainline of P*<sub>0</sub> is the list of events
+    [*P*<sub>n</sub> , ... , *P*<sub>1</sub>, *P*<sub>0</sub>],
+ordered from oldest to newest.


The implicit reference to time in the form of "oldest and newest" is throwing me here a bit. Perhaps this?

Suggested change

ordered from oldest to newest.

ordered such that the indices are descending, so that the event we started with is last.

For what it's worth: each $P_i$ cites $P_{i+1}$ as an auth event. So the latter is "older" in some sense... but it's not necessarily "older" in the sense implied by the prev_events edges, or "older" as determined by origin_server_ts or even received_ts. So I agree that "older" is more trouble than it's worth.

However: now that we have explicit indices that the text later refers to, I'd be tempted to just write it out starting with $P_0$ and let indices increase.

I think this comment thread is outdated. The current wording includes explicit indexes and is clear for me.

content/rooms/fragments/v2-state-res.md

Co-authored-by: Denis Kasak <[email protected]>

content/rooms/fragments/v2-state-res.md

Co-authored-by: Denis Kasak <[email protected]>

richvdh

much improved for sure! Some minor suggestions

content/rooms/fragments/v2-state-res.md

changelogs/room_versions/newsfragments/1107.clarification

Co-authored-by: Travis Ralston <[email protected]>

Co-authored-by: Richard van der Hoff <[email protected]>

content/rooms/fragments/v2-state-res.md

Co-authored-by: Richard van der Hoff <[email protected]>

addressed

content/rooms/fragments/v2-state-res.md

richvdh

lgtm!

David Robertson added 3 commits June 1, 2022 15:25

Better description of the mainline of a PL event.

4544019

Opinionated: improve def of the closest event to e

e2d8cfc

``iteratively descending through the `m.room.power_levels` events in the `auth_events`-starting at *e*.`` was also unclear to me. Unfortunately this is now more symbol-dense and I'm not sure if it's a net improvement.

Changelog

8d6e328

DMRobertson marked this pull request as ready for review June 1, 2022 14:59

DMRobertson requested a review from a team as a code owner June 1, 2022 14:59

turt2live self-requested a review June 1, 2022 15:34

dkasak reviewed Jun 2, 2022

View reviewed changes

DMRobertson marked this pull request as draft June 5, 2022 23:39

DMRobertson and others added 3 commits June 6, 2022 12:50

correct spelling of power_levels

91da0e1

Co-authored-by: Denis Kasak <[email protected]>

Rework the mainline ordering more dramatically.

c2d55ee

This has undergone a fairly large rework, so needs careful inspection.

Minor typographical tweaks

4ecd8a8

looks better in Firefox. I am sad to be writing a poor man's TeX though.

DMRobertson marked this pull request as ready for review June 6, 2022 13:27

DMRobertson requested a review from dkasak June 6, 2022 13:27

dkasak approved these changes Jun 8, 2022

View reviewed changes

Batch of suggestions, thanks Denis

dbf66b9

Co-authored-by: Denis Kasak <[email protected]>

DMRobertson commented Jun 8, 2022

View reviewed changes

content/rooms/fragments/v2-state-res.md Outdated Show resolved Hide resolved

DMRobertson commented Jun 8, 2022

View reviewed changes

content/rooms/fragments/v2-state-res.md Outdated Show resolved Hide resolved

Note that e_0 may not be in the second list; typo fix

ac5c477

Co-authored-by: Denis Kasak <[email protected]>

richvdh approved these changes Jun 14, 2022

View reviewed changes

turt2live previously requested changes Jun 14, 2022

View reviewed changes

changelogs/room_versions/newsfragments/1107.clarification Outdated Show resolved Hide resolved

DMRobertson and others added 4 commits June 14, 2022 17:19

Update changelogs/room_versions/newsfragments/1107.clarification

c411d25

Co-authored-by: Travis Ralston <[email protected]>

say "mainline position" rather than "position in the mainline of P"

fe979ee

Co-authored-by: Richard van der Hoff <[email protected]>

Another "position in the mainline" -> "mainline position"

5945937

Co-authored-by: Richard van der Hoff <[email protected]>

Enumerate the mainline with ascending indices; drop older/newer comment

9231ca6

DMRobertson requested a review from richvdh June 14, 2022 17:17

richvdh reviewed Jun 14, 2022

View reviewed changes

content/rooms/fragments/v2-state-res.md Outdated Show resolved Hide resolved

content/rooms/fragments/v2-state-res.md Outdated Show resolved Hide resolved

content/rooms/fragments/v2-state-res.md Outdated Show resolved Hide resolved

Explicitly say that we want mainline positions from P

e2f3895

Co-authored-by: Richard van der Hoff <[email protected]>

More fixes.

00e225b

Co-authored-by: Richard van der Hoff <[email protected]>

DMRobertson commented Jun 14, 2022

View reviewed changes

content/rooms/fragments/v2-state-res.md Outdated Show resolved Hide resolved

Indentation fix

f243e8b

DMRobertson requested a review from richvdh June 15, 2022 14:55

richvdh approved these changes Jun 15, 2022

View reviewed changes

richvdh merged commit 2ee2172 into matrix-org:main Jun 15, 2022

zecakeh mentioned this pull request Jun 17, 2022

Incorporate Matrix v1.3 changes ruma/ruma#1145

Closed

66 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better description of the mainline of a PL event. #1107

Better description of the mainline of a PL event. #1107

DMRobertson commented Jun 1, 2022 •

edited by github-actions bot

Loading

DMRobertson commented Jun 1, 2022

dkasak Jun 2, 2022 •

edited

Loading

DMRobertson Jun 5, 2022

DMRobertson Jun 6, 2022 •

edited

Loading

dkasak Jun 6, 2022

DMRobertson Jun 6, 2022

DMRobertson commented Jun 6, 2022

DMRobertson commented Jun 6, 2022

dkasak left a comment

dkasak Jun 8, 2022

DMRobertson Jun 8, 2022

richvdh Jun 14, 2022

richvdh left a comment

richvdh left a comment

	ordered from oldest to newest.
	ordered such that the indices are descending, so that the event we started with is last.

Better description of the mainline of a PL event. #1107

Better description of the mainline of a PL event. #1107

Conversation

DMRobertson commented Jun 1, 2022 • edited by github-actions bot Loading

DMRobertson commented Jun 1, 2022

dkasak Jun 2, 2022 • edited Loading

Choose a reason for hiding this comment

DMRobertson Jun 5, 2022

Choose a reason for hiding this comment

DMRobertson Jun 6, 2022 • edited Loading

Choose a reason for hiding this comment

dkasak Jun 6, 2022

Choose a reason for hiding this comment

DMRobertson Jun 6, 2022

Choose a reason for hiding this comment

DMRobertson commented Jun 6, 2022

DMRobertson commented Jun 6, 2022

dkasak left a comment

Choose a reason for hiding this comment

dkasak Jun 8, 2022

Choose a reason for hiding this comment

DMRobertson Jun 8, 2022

Choose a reason for hiding this comment

richvdh Jun 14, 2022

Choose a reason for hiding this comment

richvdh left a comment

Choose a reason for hiding this comment

richvdh left a comment

Choose a reason for hiding this comment

DMRobertson commented Jun 1, 2022 •

edited by github-actions bot

Loading

dkasak Jun 2, 2022 •

edited

Loading

DMRobertson Jun 6, 2022 •

edited

Loading