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

Fix basic requirements of proof #166

Merged
merged 1 commit into from
Aug 24, 2023
Merged

Fix basic requirements of proof #166

merged 1 commit into from
Aug 24, 2023

Conversation

TallTed
Copy link
Member

@TallTed TallTed commented Aug 14, 2023

Comment on lines +535 to +536
<dfn class="lint-ignore">`proof`</dfn> property MUST be used. If present, its
value MUST be either a single object, or an unordered set of objects, expressed
Copy link
Member

@msporny msporny Aug 17, 2023

Choose a reason for hiding this comment

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

Suggested change
<dfn class="lint-ignore">`proof`</dfn> property MUST be used. If present, its
value MUST be either a single object, or an unordered set of objects, expressed
<dfn class="lint-ignore">`proof`</dfn> property MUST be used and its
value MUST be either a single object, or an unordered set of objects, expressed

It's a bit strange to say "If present" for a property that MUST be present? Minor rewording suggestions.

Copy link
Member Author

@TallTed TallTed Aug 17, 2023

Choose a reason for hiding this comment

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

@msporny @dlongley -- Reading the whole paragraph, it's only a MUST "When expressing a data integrity proof on an object", so "if present" seems OK.

I think the "If present" phrasing covers any time the proof property is present, while the "and" phrasing only covers the proof property "When expressing a data integrity proof on an object".

Some additional rephrasing, possibly including other segments of the doc, may be needed to make the restrictions on the proof property clearer.

Copy link
Member

@msporny msporny Aug 17, 2023

Choose a reason for hiding this comment

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

If you feel strongly about this, I'm fine w/ leaving it as is. I don't think the new wording I suggested changes the meaning of the sentence.

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer as-is, because this doesn't spark the need for further review/rewording.

Copy link
Contributor

@dlongley dlongley left a comment

Choose a reason for hiding this comment

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

Approving w/@msporny's suggestion.

@msporny msporny merged commit 1aa1735 into w3c:main Aug 24, 2023
1 check passed
@TallTed TallTed deleted the main-1 branch August 25, 2023 01:07
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.

4 participants