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

Reword & re-link zkvm README #370

Merged
merged 2 commits into from
Feb 8, 2023
Merged

Conversation

tzerrell
Copy link
Member

@tzerrell tzerrell commented Feb 7, 2023

Since the crate-level readmes appear on both crates.io and docs.rs, reword and adjust the links to be appropriate for both. (Example of the status quo, showing the broken links and misleading language on crates.io: https://crates.io/crates/risc0-zkvm/0.12.0)

@tzerrell tzerrell requested review from pdg744 and 3lkn February 7, 2023 22:51
@tzerrell tzerrell self-assigned this Feb 7, 2023
Copy link
Contributor

@pdg744 pdg744 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!

LGTM, though I'm not really sure how to gauge whether the crate references are correct.
Is there a way I can locally preview the revisions to check the links?

@pdg744
Copy link
Contributor

pdg744 commented Feb 7, 2023

Perhaps we should add one or more of the crates.io/docs.rs pages to the link-checker script?

@tzerrell
Copy link
Member Author

tzerrell commented Feb 7, 2023

The crates.io/docs.rs pages only update when we publish a release and so can't be directly checked (including by script).

The docs can be built locally (cargo doc), and I did check my link changes there and they seem to be working ok. That said, the local environment does look different from the published environment in a way that can break links (and has for us in the past). I don't think I did anything that should break but that's not the same as actually publishing and testing.

Edit: Tagging @flaub in case he knows of some tool or technique I'm unaware of.

@pdg744
Copy link
Contributor

pdg744 commented Feb 7, 2023

The crates.io/docs.rs pages only update when we publish a release and so can't be directly checked (including by script).

We can run a script on any website to check for broken links, no?
My comment was meant as as a suggestion to surface dead links, not as a means to check the links in the PR.

@tzerrell
Copy link
Member Author

tzerrell commented Feb 7, 2023

We can run a script on any website to check for broken links, no? My comment was meant as as a suggestion to surface dead links, not as a means to check the links in the PR.

Ah, fair enough. We'd have to adjust some settings (if I understand what recursiveLinks does correctly it would be a bad idea in this context), and since there's only anything to check after a release is pushed we'd rarely run it (perhaps even only manually? Or we could just do weekly and not worry that most weeks are redundant).

@flaub
Copy link
Member

flaub commented Feb 8, 2023

I don't have any good tools for link checking of docs off the top of my head.

@tzerrell tzerrell merged commit f870c87 into main Feb 8, 2023
@tzerrell tzerrell deleted the tzerrell/readme-links-for-crates-io branch February 8, 2023 00:30
@tzerrell
Copy link
Member Author

tzerrell commented Feb 8, 2023

Landing as is -- @pdg744's link checker suggestion could probably be useful for our crates.io pages, although I'm don't see how it would be effective for our docs.rs pages (since I believe we have to turn recursion off and they have many pages). Opening #371 for that possibility.

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