Skip to content

Conversation

@jswrenn
Copy link
Collaborator

@jswrenn jswrenn commented Mar 5, 2024

Since cargo deadlinks is just a link-checking wrapper around cargo doc, we can simply replace our doc invocation with deadlinks to get the benefits of both.

# is a link-checking wrapper around `cargo doc`.
export RUSTDOCFLAGS="${{ matrix.toolchain == 'nightly' && '-Z unstable-options --document-hidden-items' || '' }} $RUSTDOCFLAGS $METADATA_DOCS_RS_RUSTDOC_ARGS"
./cargo.sh +${{ matrix.toolchain }} deadlinks --check-intra-doc-links -- --document-private-items --package ${{ matrix.crate }} ${{ matrix.features }}
if: matrix.toolchain != 'msrv'
Copy link
Member

Choose a reason for hiding this comment

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

Explain why?

Also, I'd recommend just nightly. Runs fewer jobs and the behavior of this is very unlikely to differ by toolchain. Notably, our docs.rs docs are only built from nightly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The MSRV of this version of deadlinks is higher than our MSRV.

Restricting to nightly seems reasonable, though.

Comment on lines 277 to 278
# We invoke `cargo deadlinks` instead of `cargo doc`; `cargo deadlinks`
# is a link-checking wrapper around `cargo doc`.
Copy link
Member

Choose a reason for hiding this comment

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

No we don't anymore 🙂

@jswrenn jswrenn force-pushed the deadlinks branch 3 times, most recently from 61ac2c7 to 2022b0c Compare March 5, 2024 16:06
Since `cargo deadlinks` is just a link-checking wrapper around
`cargo doc`, we can simply replace our `doc` invocation with
`deadlinks` to get the benefits of both.
@jswrenn
Copy link
Collaborator Author

jswrenn commented Mar 5, 2024

Hm, I can't replicate the issue here locally: https://github.com/google/zerocopy/actions/runs/8159490578/job/22304041953?pr=1021

Copy link
Member

@joshlf joshlf left a comment

Choose a reason for hiding this comment

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

- name: Check dead links
run: |
cargo install -q cargo-deadlinks --version 0.8.1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cargo install -q cargo-deadlinks --version 0.8.1
set -eo pipefail
cargo install -q cargo-deadlinks --version 0.8.1

And honestly, in other jobs (notably, the previous one) while you're at it. We should have it in every job.

@joshlf joshlf mentioned this pull request Sep 20, 2024
87 tasks
@joshlf joshlf added the do-after-next-release Not blocking release, but we should do soon after release label Oct 2, 2024
joshlf added a commit that referenced this pull request Oct 5, 2024
This is a point in favor of landing #1021
@joshlf joshlf mentioned this pull request Oct 5, 2024
joshlf added a commit that referenced this pull request Oct 5, 2024
Release 0.8.2.

This is a point in favor of landing #1021
github-merge-queue bot pushed a commit that referenced this pull request Oct 5, 2024
Release 0.8.2.

This is a point in favor of landing #1021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-after-next-release Not blocking release, but we should do soon after release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants