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

Proof reading for Section 8 - Privacy #1554

Merged
merged 5 commits into from
Sep 15, 2024
Merged

Proof reading for Section 8 - Privacy #1554

merged 5 commits into from
Sep 15, 2024

Conversation

decentralgabe
Copy link
Contributor

@decentralgabe decentralgabe commented Aug 26, 2024

Proof reads for Section 8
Also included @TallTed's changes from #1549

Section 9 coming next


Preview | Diff

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.

Browser crash ate my PR for sections 1-4. I'll try to do that again, but thought it better to get this batch of change requests done first.

README.md Outdated Show resolved Hide resolved
terms.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@decentralgabe
Copy link
Contributor Author

thanks @TallTed for the thorough review. I've incorporated your changes.

@TallTed
Copy link
Member

TallTed commented Aug 27, 2024

@decentralgabe — Going forward, whenever possible, it's better to use the "Add suggestion to batch" buttons, as these automatically "resolve conversation" and are certain to capture all elements of a given suggestion (including sometimes overlooked punctuation and other single-character changes within a larger block). Here and now, I need to review all my suggestions against the current state of the PR, so as to be sure no such overlooks have happened.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@decentralgabe
Copy link
Contributor Author

added the remaining edits and did a pass for line breaks for the doc

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

TallTed commented Aug 27, 2024

To successfully review my 90+ change requests in reasonable timeframe, I have to mark them all as unresolved, and then mark each resolved as I confirm it's been applied. That's what I'm starting on now.

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.

Resolving the 7 or so new suggestions should complete my changes for this PR. I still intend to re-review at least sections 1–4, as I know there were some important (though, I believe, only editorial) fixes in what I was drafting.

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

iherman commented Sep 1, 2024

The issue was discussed in a meeting on 2024-08-28

  • no resolutions were taken
View the transcript

3.4. Proof reading for Section 8 - Privacy (pr vc-data-model#1554)

See github pull request vc-data-model#1554.

Brent Zundel: this is gabe's review of the privacy section.

Gabe Cohen: this is just privacy section, I'll do security next.

Brent Zundel: lots of comments, feedback incorporated. I encourage folks to look at the PR.
… anything else, related to VC DM?

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

iherman commented Sep 11, 2024

The issue was discussed in a meeting on 2024-09-11

  • no resolutions were taken
View the transcript

3.2. Proof reading for Section 8 - Privacy (pr vc-data-model#1554)

See github pull request vc-data-model#1554.

Brent Zundel: editorial overview of the privacy considerations section done by gabe.

Manu Sporny: 100% OK with the PR before but then the abstract for the document got changed. The group has spent a lot of time getting the abstract language right, please look at the new abstract and decide if you prefer the new language.

Ted Thibodeau Jr.: an abstract is supposed to roughly summarize the whole document, not introduce you to the document. The current abstract is replicated 100% in the intro that follows. It is fine in the intro but not an abstract, so that is why I made the changes.

Manu Sporny: can we break that PR into a different PR so we can get the privacy changes in then discuss the abstract changes?

Joe Andrieu: +1 for a separate PR. I like updating the abstract, but let's separate.

Manu Sporny: the new abstract might be inscrutable to someone not familiar with the document.

Ted Thibodeau Jr.: sure, can split PRs.

Brent Zundel: any other comments on this PR?

Ted Thibodeau Jr.: currently the abstract also exists 100% in the intro, in my change requests it has been changed in the intro but retained as the intro, if we are going to do the pull out of the abstract change, my revised paragraph should move to replace the existing abstract, any objection?

Joe Andrieu: +1 to that TallTed.

Brent Zundel: the fact that the abstract is a problem is a separate issue from privacy considerations, would like to see that resolved in separate PR.

Ted Thibodeau Jr.: I think this PR is broader than that but will see what I can do.
… it is broader than that, the initial comment on this is not proofreading privacy, it is that plus changes from a previous PR which is more broadly scoped.

Brent Zundel: thank you for the clarity, but my desire remains for a separate PR.

Ted Thibodeau Jr.: I will put my big block on the abstract in a separate PR, the remaining changes are to the introduction.

Manu Sporny: no issue with you doing that with a separate PR, Gabe started with privacy section and then began modifying other sections of the document.
… makes PR less focused.
… the current concern is that there are no objections to other text, but I am objecting to changes to abstract and introduction.
… I agree that there should be differences between abstract and intro, no issue with preserving what you have written into the introduction, needs to be in separate PR so the group can wordsmith.

Ted Thibodeau Jr.: I will adjust my change suggestions in this PR for the abstract and the intro.

Brent Zundel: moving to issues.

@decentralgabe
Copy link
Contributor Author

@msporny with the abstract piece reverted this should be good to go

@msporny
Copy link
Member

msporny commented Sep 15, 2024

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

@msporny msporny merged commit 0ad7383 into main Sep 15, 2024
1 check passed
@msporny msporny deleted the proof-reading branch September 15, 2024 21:43
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