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

Switch build to GitHub action using spec-prod #262

Merged
merged 6 commits into from
Mar 3, 2021
Merged

Conversation

tidoust
Copy link
Member

@tidoust tidoust commented Mar 2, 2021

This creates a GitHub action that replaces the Travis CI worfklow for re-generating the specification and updating the gh-pages branch.

The action uses w3c/spec-prod under the hoods:
https://github.com/w3c/spec-prod/#spec-prod

w3c/spec-prod takes care of everything and also validates the spec (markup and broken hyperlinks).

Good thing about this approach is that no specific secret tokens are needed, since GitHub actions are allowed to push to the gh-pages branch. Also, no need to maintain a complex deploy script.


Preview | Diff

This creates a GitHub action that replaces the Travis CI worfklow for
re-generating the specification and updating the `gh-pages` branch.

The action uses w3c/spec-prod under the hoods:
https://github.com/w3c/spec-prod/#spec-prod

w3c/spec-prod takes care of everything and also validates the spec (markup and
broken hyperlinks).

Good thing about this approach is that no specific secret tokens are needed,
since GitHub actions are allowed to push to the gh-pages branch. Also, no need
to maintain a complex deploy script.
There was a typo in one of the links to the capabilities registry, which makes
the validation fail.

Also, GitHub does not serve Markdown files as HTML files on GitHub Pages by
default. This update switches these links to `github.com` URLs.
@tidoust tidoust marked this pull request as draft March 2, 2021 19:00
@tidoust
Copy link
Member Author

tidoust commented Mar 2, 2021

So, build currently fails because HTML validation fails. Markup is invalid due to the CDDL messages appendix that has the structure <pre>... <p> </p> </pre>, and <p> is not allowed in <pre>. I do not know yet where these <p> are created, since they do not exist in messages_appendix.html. I'll investigate (this needs to be fixed for publication as First Public Working Draft) and will update the PR accordingly.

See: https://tabatkins.github.io/bikeshed/#including-raw

This avoids the HTML parsing that Bikeshed does and wrapping of paragraphs in
`<p>` tags.
@tidoust tidoust marked this pull request as ready for review March 2, 2021 19:09
@tidoust tidoust requested a review from markafoltz March 2, 2021 19:17
@tidoust
Copy link
Member Author

tidoust commented Mar 2, 2021

OK, I found the right include setting in Bikeshed to avoid the introduction of <p> tags.

All things should work now and this PR should fix #259.

Copy link
Contributor

@markafoltz markafoltz 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 making these changes; it would be helpful for me to document the referenced actions/workflows for future maintenance :-)

.github/workflows/pr-push.yml Show resolved Hide resolved
.github/workflows/pr-push.yml Show resolved Hide resolved
index.bs Outdated
@@ -195,7 +195,8 @@ Non-Functional Requirements {#requirements-non-functional}
1. It should be possible to implement an OSP agent using modest
hardware requirements, similar to what is found in a low end smartphone,
smart TV or streaming device. See the [Device
Specifications](device_specs.md) document for agent hardware specifications.
Specifications](https://github.com/w3c/openscreenprotocol/blob/master/device_specs.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe these links should point to the gh-pages branch since we are serving the spec document from that branch?

I don't feel strongly since the two branches should always be kept in sync by the workflow.

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 would keep the gh-pages name as a "hidden" one: no need to expose the name of the branch that gets published as GitHub Pages, to avoid breaking links if we change the name. Actually, that applies to master as well, which we'll soon need to rename to main (we're progressively transitioning all W3C repositories to use main as the default branch instead of master).

I thought something had to be done for GitHub to publish Markdown files as HTML but I was wrong. It's all being done by default, you just have to replace the .md extension with .html. I pushed an update that targets the HTML pages under w3c.github.io (i.e. capabilities.html and device_specs.html). We can always choose a different theme and adjust the header and footer on these pages later on if we feel that's needed.

@markafoltz
Copy link
Contributor

Looks good, and thanks again @tidoust. Feel free to merge this when you are ready.

@tidoust tidoust merged commit 684e1e5 into master Mar 3, 2021
github-actions bot added a commit that referenced this pull request Mar 3, 2021
…his creates a GitHub action that replaces the Travis CI worfklow forre-generating the specification and updating the `gh-pages` branch.The action uses w3c/spec-prod under the hoods:https://github.com/w3c/spec-prod/#spec-prodw3c/spec-prod takes care of everything and also validates the spec (markup andbroken hyperlinks).Good thing about this approach is that no specific secret tokens are needed,since GitHub actions are allowed to push to the gh-pages branch. Also, no needto maintain a complex deploy script.This update also fixes broken links and markup validation issues.

SHA: 684e1e5
Reason: push, by @tidoust

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@markafoltz markafoltz deleted the drop-travis branch October 25, 2021 18:57
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.

2 participants