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

Add mdBook stub #527

Merged
merged 10 commits into from
Apr 28, 2023
Merged

Add mdBook stub #527

merged 10 commits into from
Apr 28, 2023

Conversation

tzerrell
Copy link
Member

@tzerrell tzerrell commented Apr 21, 2023

This PR starts a new mdBook in docs/mdbook, and adds it to CI.

Currently this is a stub. Also, CI includes running tests (although there is nothing to test yet), but does not include a deployment, on the principle that we shouldn't deploy something that doesn't have content, and that even as we add content it will be ugly for a while. I did write a short list of links to other content for the stub, so if we think it's important to be deploying this from day 1 we could set that up and not confuse our users too badly. It does deploy successfully locally with mdbook serve --open.

CI appears to be working fine, giving the same output as on my local machine, but I'm interested in any feedback about whether organizing this into the doc check was the right approach.

@tzerrell tzerrell self-assigned this Apr 21, 2023
@tzerrell tzerrell requested review from flaub and mothran April 21, 2023 22:38
@tzerrell tzerrell marked this pull request as ready for review April 21, 2023 22:38
with:
crate: mdbook
version: "0.4"
- run: mdbook test
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good, might be good to add a single test to verify its working as expected.

We could look into auto publishing (on main or on-release workflows): https://github.com/peaceiris/actions-mdbook Because we already have a GH token to publish to the benchmarks repo which has a public HTML page: https://risc0.github.io/benchmarks/

Copy link
Member

Choose a reason for hiding this comment

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

Agreed and then we could probably just rename benchmarks to something more generic, "assets", "internal-website", ???.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, it got overloaded when I started pushing crate-validator results there. I had it on my TODO to rename that repo but required changing a few other things. I can probably do that sooner though if we land on a name. generated-content might work?

Copy link
Member Author

Choose a reason for hiding this comment

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

The only reason I was leaving out auto-publishing was to "not include a deployment, on the principle that we shouldn't deploy something that doesn't have content" -- but it's not like this has content we don't want anyone to see. So I'll look into doing auto publishing, thanks for the references!

runs-on: ubuntu-latest
steps:
- name: Setup mdBook
uses: peaceiris/actions-mdbook@v1
Copy link
Contributor

Choose a reason for hiding this comment

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

TBD: review the code and fork it to risc0 org.

@tzerrell
Copy link
Member Author

Based on in-person discussion my plan is to roll back the deployment portion of this PR and split it off into its own PR. Then this can land before we've fully sorted out the deployment story.

@flaub flaub enabled auto-merge (squash) April 28, 2023 22:31
@flaub flaub merged commit 48a01ec into main Apr 28, 2023
8 checks passed
@flaub flaub deleted the tzerrell/mdbook branch April 28, 2023 23:21
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.

None yet

3 participants