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

Initial draft of the Open Screen Application Protocol. #344

Merged
merged 6 commits into from
Sep 20, 2024

Conversation

markafoltz
Copy link
Contributor

Initial draft of the application side of the spec, following plan in #341.

Copy link
Contributor

@backkem backkem left a comment

Choose a reason for hiding this comment

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

This is a clean initial split.

It may be worth considering if part of the agent-info concerns are better handled on the 'network' side of the split. E.g.: it's hard to know what you're connecting to/authenticating with without giving it a proper name. But this can be refined in iterations.

application.bs Outdated Show resolved Hide resolved
Copy link
Member

@tidoust tidoust left a comment

Choose a reason for hiding this comment

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

I just spotted a couple of metadata updates to do.
I haven't reviewed the entire application.bs file but that seems a good start :)

.github/workflows/auto-publish.yml Outdated Show resolved Hide resolved
application.html Outdated Show resolved Hide resolved
application_messages.html Outdated Show resolved Hide resolved
application.bs Outdated Show resolved Hide resolved
application.bs Outdated Show resolved Hide resolved
application.bs Outdated Show resolved Hide resolved
@markafoltz
Copy link
Contributor Author

It may be worth considering if part of the agent-info concerns are better handled on the 'network' side of the split. E.g.: it's hard to know what you're connecting to/authenticating with without giving it a proper name. But this can be refined in iterations.

@backkem I noted this while working on the network side of the spec and filed #346, will post your comment there.

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 @markafoltz for getting the split drafts out ahead the meeting.

I'd be supportive of merging this and #347 with an Editor's Draft status to facilitate review. The group decision time is when we want to publish to TR. The local builds are clean, but it'd be convenient to be able to hand out URLs to rendered documents to reviewers outside the group as well, and deploy would enable that.

@tidoust build/validate/deploy for multiple specs since spec-prod v2.3.0 do work and the workflows look good to me, thanks for tweaking those. As a secondary non-blocking consideration: PR Preview does not yet support repos with multiple specs tobie/pr-preview#18 -- are you aware of workarounds (besides separate repos)? Maybe we can live without PR Preview for a while...

@markafoltz markafoltz merged commit 659582c into main Sep 20, 2024
2 checks passed
@markafoltz markafoltz deleted the osp-application-protocol branch September 20, 2024 18:51
github-actions bot added a commit that referenced this pull request Sep 20, 2024
SHA: 659582c
Reason: push, by markafoltz

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@anssiko
Copy link
Member

anssiko commented Sep 20, 2024

@tidoust application.html deploy to gh-pages was skipped resulting in https://w3c.github.io/openscreenprotocol/application.html

Build and validate passed.

Looking at https://github.com/w3c/spec-prod/blob/main/docs/examples.md#multiple-specs-in-same-repository I believe a possible fix is to add this line:
DESTINATION: application.html

After this line:

(And similarly for the network part.)

@tidoust
Copy link
Member

tidoust commented Sep 23, 2024

I believe a possible fix is to add this line: DESTINATION: application.html

That may work but, if it does, I don't understand why ;)
DESTINATION should take the value application.html all by itself when SOURCE is application.bs.

I'm looking into it but don't know yet why the job skipped deployment to gh-pages.

@tidoust
Copy link
Member

tidoust commented Sep 23, 2024

I'm looking into it but don't know yet why the job skipped deployment to gh-pages.

Actually, that's easy, we're just not looking at the right logs ;) These logs are for the PR itself, where the spec-prod action validates things but indeed does not deploy anything (on purpose, that's a PR). The right logs to look show a Bikeshed validation error because nothing build application_messages.html. That's what needs fixing. I'll prepare a PR.

@tidoust
Copy link
Member

tidoust commented Sep 23, 2024

The right logs to look show a Bikeshed validation error because nothing build application_messages.html

More accurately, the file gets generated, but the action resets the status of the git folder after taking care of the first document (through a call to git clean), which deletes the other generated HTML files and makes the following generation of application.html fail. The action works best in completely separate jobs. Anyway, I'll propose a rewrite.

@tidoust
Copy link
Member

tidoust commented Sep 23, 2024

Publication now fixed. Both drafts are now available at:
https://w3c.github.io/openscreenprotocol/application.html
https://w3c.github.io/openscreenprotocol/network.html

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