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

Refine spec concepts, terminology, prose, definitions, and algorithms. #87

Merged
merged 9 commits into from Feb 11, 2022

Conversation

michaelwasserman
Copy link
Member

@michaelwasserman michaelwasserman commented Feb 10, 2022

Hey @reillyeon, please excuse the size of this change and take a look; thanks!
I tried to incorporate first pass changes to address your last round of feedback and #79, #80, and #81.

FYI @tomayac, @anssiko, @pwnall, @inexorabletash, @quisquous for highly optional review. Your feedback would be appreciated, but please don't feel obligated. thanks!


Preview | Diff

Copy link
Member

@anssiko anssiko left a comment

Choose a reason for hiding this comment

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

Thank you @michaelwasserman!

I approve this PR with my Second Screen WG chair hat on. I'm not conducting a line-by-line review but the question I'm considering is:

"Does this spec satisfy the technical requirement expected of a W3C Working Group deliverable?"

My assessment is this spec meets or exceeds those requirements. This update further improves the spec by aligning it with established conventions. In particular I'm happy to see both security and privacy considerations carefully written down.

I think this spec is almost First Public Working Draft ready. The revised WG charter is currently in AC review, and if the review completes without objections and with adequate support I'm projecting we can bring this work to the WG end of Q1 or early Q2 to continue refine it further with more contributors and wider review.

Copy link
Contributor

@tomayac tomayac left a comment

Choose a reason for hiding this comment

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

LGTM with minor nits.

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
Co-authored-by: Thomas Steiner <tomac@google.com>
Copy link
Member Author

@michaelwasserman michaelwasserman left a comment

Choose a reason for hiding this comment

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

Thanks for the suggestions, @tomayac! I'm commenting to modify some of your suggestions.

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
@michaelwasserman
Copy link
Member Author

Thanks for the great feedback, @tomayac!
Thanks for the assessment and WG context, @anssiko!

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
Co-authored-by: Reilly Grant <reillyeon@users.noreply.github.com>
Copy link
Member Author

@michaelwasserman michaelwasserman left a comment

Choose a reason for hiding this comment

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

Thank you for the feedback, @reillyeon!

I applied your suggestions and offered my own to address your other comments here. I'll apply those and merge the PR momentarily. I'm happy to follow up with any additional feedback in subsequent PRs.

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
@michaelwasserman michaelwasserman merged commit dcf7c28 into main Feb 11, 2022
@michaelwasserman michaelwasserman deleted the spec_refinements branch February 11, 2022 06:33
index.bs Show resolved Hide resolved
github-actions bot added a commit that referenced this pull request Feb 11, 2022
#87)

SHA: dcf7c28
Reason: push, by @michaelwasserman

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
bradtriebwasser pushed a commit to bradtriebwasser/window-placement that referenced this pull request Oct 19, 2022
w3c#87)

* Make substantial refinements to the spec

Co-authored-by: Mike Wasserman <msw@chromium.org>
Co-authored-by: Thomas Steiner <tomac@google.com>
Co-authored-by: Reilly Grant <reillyeon@users.noreply.github.com>
bradtriebwasser pushed a commit to bradtriebwasser/window-placement that referenced this pull request Oct 19, 2022
w3c#87)

* Make substantial refinements to the spec

Co-authored-by: Mike Wasserman <msw@chromium.org>
Co-authored-by: Thomas Steiner <tomac@google.com>
Co-authored-by: Reilly Grant <reillyeon@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants