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

Controller of Verification Method #139

Merged
merged 5 commits into from
Jan 12, 2025
Merged

Controller of Verification Method #139

merged 5 commits into from
Jan 12, 2025

Conversation

David-Chadwick
Copy link
Contributor

@David-Chadwick David-Chadwick commented Jan 3, 2025

Clarify that it is an assertion and not necessarily a statement of fact


Preview | Diff

Clarify that it is an assertion and not necessarily a statement of fact
index.html Outdated Show resolved Hide resolved
Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

These two changes go together. They are only separated because GitHub won't take a suggestion that applies across a changed line.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
David-Chadwick and others added 2 commits January 5, 2025 09:38
Co-authored-by: Ted Thibodeau Jr <[email protected]>
as opposed to asserted controller
index.html Outdated Show resolved Hide resolved
Co-authored-by: Ted Thibodeau Jr <[email protected]>
@David-Chadwick
Copy link
Contributor Author

add forward pointer to security section

@iherman
Copy link
Member

iherman commented Jan 8, 2025

The issue was discussed in a meeting on 2025-01-08

  • no resolutions were taken
View the transcript

3.2. Controller of Verification Method (pr cid#139)

See github pull request cid#139.

Brent Zundel: Ok, jump on the queue for other comments or we will move to #139.
… This PR has been open for five days, one request for changes from Ted, otherwise it's a pretty small PR.

Manu Sporny: +1 in general to the PR, the PR makes two changes, the first one is not necessary because we already say it in the spec. David and I talked a bit about the PR over the break. The first change says you can't trust a controller of a document to truthfully say which VMs they have control over.
… You have to follow the VM URL to get its controlled identifier document and check the bidirectional relationship.
… I think we'd be restating it a third time but I need to get a reference for David to see if he agrees.
… So maybe some refinement but +1 to the general direction.

David Chadwick: I think what Manu says is correct, but it seems to me that the comment that you first addressed that it should be made at the time that the controller is introduced for VMs and an example is different -- rather than stating it at the end in the security section.
… Ted made some positive changes which I accepted apart from wording. So when it says there are outstanding comments but I don't believe it is. I don't know what to do there -- just need Ted to respond.

Ted: I resolved them.

David Chadwick: Thanks Ted.

Brent Zundel: Thanks Ted.

Manu Sporny: I'm not opposed to keeping the first paragraph in, your reasoning works for me but can you add a reference to the security consideration section? Then people can click through and its all linked, etc.

David Chadwick: Sure, no problem.

Brent Zundel: I was going to ask for the same thing.
… That was the PRs on the controlled identifiers spec, we have a number of open issues still.
… We're going to go through them.

index.html Outdated Show resolved Hide resolved
@msporny
Copy link
Member

msporny commented Jan 12, 2025

Editorial, multiple reviews, changes requested and made, no objections, merging.

@msporny msporny merged commit 901c2a2 into w3c:main Jan 12, 2025
1 check passed
@David-Chadwick
Copy link
Contributor Author

I looked for an appropriate part of the security section to point to and could not find an appropriate one. I was going to report this to the group at this week's meeting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants