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

Actually fail CI for doc warnings #32

Merged
merged 1 commit into from
Mar 26, 2020

Conversation

CAD97
Copy link
Collaborator

@CAD97 CAD97 commented Mar 25, 2020

This should fail without a further PR to fix the broken intra doc link, if this is doing what it should do.

@CAD97 CAD97 mentioned this pull request Mar 25, 2020
@CAD97
Copy link
Collaborator Author

CAD97 commented Mar 25, 2020

Ok, this is weird. Setting RUSTDOCFLAGS for me locally is definitely doing what it should do.

PS D:\repos\rust-analyzer\text-size> cargo +nightly doc
 Documenting text-size v0.99.0-dev.2 (D:\repos\rust-analyzer\text-size)
warning: `[TextSized]` cannot be resolved, ignoring it.
  --> src\size.rs:47:62
   |
47 |     /// want to be usable in this constructor can implement [`TextSized`].
   |                                                              ^^^^^^^^^^^ cannot be resolved, ignoring
   |
   = note: `#[warn(intra_doc_link_resolution_failure)]` on by default
   = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]`

    Finished dev [unoptimized + debuginfo] target(s) in 0.63s

PS D:\repos\rust-analyzer\text-size> $env:RUSTDOCFLAGS="-D warnings"

PS D:\repos\rust-analyzer\text-size> cargo +nightly doc
 Documenting text-size v0.99.0-dev.2 (D:\repos\rust-analyzer\text-size)
error: `[TextSized]` cannot be resolved, ignoring it.
  --> src\size.rs:47:62
   |
47 |     /// want to be usable in this constructor can implement [`TextSized`].
   |                                                              ^^^^^^^^^^^ cannot be resolved, ignoring
   |
   = note: `-D intra-doc-link-resolution-failure` implied by `-D warnings`
   = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]`

error: aborting due to previous error

error: Could not document `text-size`.

Caused by:
  process didn't exit successfully: `rustdoc --edition=2018 --crate-type lib --crate-name text_size src\lib.rs -o D:\.ru
st\target\doc --error-format=json --json=diagnostic-rendered-ansi -L dependency=D:\.rust\target\debug\deps -D warnings`
(exit code: 1)

PS D:\repos\rust-analyzer\text-size> git branch
  ci-docs
  docs
  huge-decimal-numbers-maybe-are-not-so-great
  master
* rustdocflags
  textsize-of
  uglify-textsized

@CAD97
Copy link
Collaborator Author

CAD97 commented Mar 25, 2020

Oh, right, that's going to be the issue. Just installing the toolchain does not make it the default, so that job is instead using the GHA-pre-installed stable toolchain.

@CAD97
Copy link
Collaborator Author

CAD97 commented Mar 26, 2020

With #33 this passes now.

@CAD97 CAD97 requested a review from matklad March 26, 2020 00:34
@CAD97
Copy link
Collaborator Author

CAD97 commented Mar 26, 2020

Alternatively, apply the further diff

diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index 4538ca8..73340d0 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -9,6 +9,7 @@ on:
 
 env:
   RUSTFLAGS: -D warnings
+  RUSTDOCFLAGS: -D warnings
   RUSTUP_MAX_RETRIES: 10
   CARGO_NET_RETRY: 10
 
@@ -51,4 +52,4 @@ jobs:
         override: true
 
     - name: Rustdoc
-      run: cargo rustdoc --all-features -- -D warnings
+      run: cargo doc --all-features

@@ -7,6 +7,11 @@ on:
- staging
- trying

env:
Copy link
Member

Choose a reason for hiding this comment

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

Wait, does this actually work? Docs don't say it does. Could you add a warning to rustc code to check this?

Copy link
Member

Choose a reason for hiding this comment

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

checked on ra, it does work 🎉

@matklad matklad merged commit 53123e5 into rust-analyzer:master Mar 26, 2020
@CAD97 CAD97 deleted the rustdocflags branch April 7, 2020 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants