Add warning if a resolution failed by GuillaumeGomez · Pull Request #49542 · rust-lang/rust

GuillaumeGomez · 2018-03-31T12:50:32Z

oli-obk · 2018-03-31T14:40:19Z

Can you add a ui test showing the new warnings?

GuillaumeGomez · 2018-03-31T15:59:22Z

Good idea!

Mark-Simulacrum · 2018-04-01T19:08:59Z

No need for this as builder.rustdoc below will take care of it for you.

cmd.arg("--rustdoc-path").arg(builder.rustdoc(compiler.host)); on what might be 898 -- that call to rustdoc will auto-ensure it for you, so you shouldn't need this.

GuillaumeGomez · 2018-04-01T20:22:36Z

Updated (I also added some useful options to rustdoc).

oli-obk · 2018-04-01T22:15:55Z

What's with this src thing?

That's actually a good question... Looking where it's coming from.

oli-obk · 2018-04-01T22:25:43Z

This is a lot of code for very few tests. I was hoping we could somehow express this in the regular ui tests by inserting some flags via comments in the test?

As would i, but (as far as i know) there's no facility in compiletest to swap out which binary is called for a specific test - only which arguments are passed. In addition, we would have to tell bootstrap to have rustdoc ready for the UI tests, even if someone is trying to run a specific non-rustdoc test.
Plus, we wanted to introduce UI tests for rustdoc at some point anyway. >_> Having them in a separate test directory will help us when we add more over time.

We need to have a struct directly to bind the test folder path.

oli-obk · 2018-04-01T22:27:23Z

Are there spans available that we can use to report the warnings?

It'll be part of another pr. I think this one is already big enough.

oli-obk · 2018-04-01T22:28:47Z

What is the purpose of these changes?

Allow rustdoc output to be checked through UI test suite.

GuillaumeGomez · 2018-04-02T16:06:37Z

Fixed a few comments.

QuietMisdreavus · 2018-04-02T21:52:05Z

It's probably worth changing the [-] references to also use backticks, just so they're all using the same style.

QuietMisdreavus · 2018-04-02T22:08:39Z

The rustdoc changes look good to me, but i'm less certain about the changes to bootstrap and compiletest. I'd like some extra eyes to look at those bits.

GuillaumeGomez · 2018-04-04T14:18:20Z

cc @oli-obk

Mark-Simulacrum · 2018-04-05T21:27:45Z

This struct should be replaceable with a host_test! macro invocation I believe.

I tried to at first to avoid writing code by hand, but failed... I can't only rely on UI or rustdoc test suite, I need a mix of the two.

Mark-Simulacrum · 2018-04-05T21:29:09Z

Avoiding these changes seems good for consistency's sake.

How would you do it?

Er, this file should be possible to leave as-is: both changes were simply extracted from struct's definition AFAICT.

Mark-Simulacrum · 2018-04-05T21:29:53Z

Leftover debugging code?

Oops indeed!

oli-obk

ups, forgot to submit this review...

oli-obk · 2018-04-04T19:03:23Z

Why isn't this deny warnings failing the test?

It hasn't been implemented in rustdoc (yet?). I'll remove this line.

oli-obk · 2018-04-04T19:04:51Z

Merge the if below into this one

oli-obk · 2018-04-04T19:06:45Z

You need to make this depend on rustdoc. Otherwise rustdoc might not be available when running the tests. A call to ensure should suffice

Apparently it's done correctly (or at least that's what I understood from @Mark-Simulacrum's comment).

Yes, this is fine as is, we depend on rustdoc below when we pass it's path.

GuillaumeGomez · 2018-04-07T16:42:44Z

Updated.

GuillaumeGomez · 2018-04-13T21:11:37Z

Anyone? cc @oli-obk @Mark-Simulacrum @QuietMisdreavus

Mark-Simulacrum · 2018-04-14T00:01:32Z

r=me on bootstrap changes

GuillaumeGomez · 2018-04-14T10:28:17Z

@bors: r=Mark-Simulacrum

bors · 2018-04-14T10:28:18Z

📌 Commit ae4c6d0 has been approved by Mark-Simulacrum

bors · 2018-04-14T15:42:55Z

🔒 Merge conflict

bors · 2018-04-15T18:16:29Z

⌛ Testing commit 4b7f4cc with merge f6d0bfd...

@QuietMisdreavus

…Mark-Simulacrum Add warning if a resolution failed r? @QuietMisdreavus

bors · 2018-04-15T21:18:33Z

💔 Test failed - status-appveyor

GuillaumeGomez · 2018-04-15T21:44:05Z

The appveyor tests output just end on running 692 tests. So I have no idea if it's a "normal" failure or not...

bors · 2018-04-16T07:54:41Z

☔ The latest upstream changes (presumably #49956) made this pull request unmergeable. Please resolve the merge conflicts.

kennytm · 2018-04-16T09:19:24Z

@GuillaumeGomez That looks like just a spurious 3-hour timeout. But now you need to rebase anyway...

GuillaumeGomez · 2018-04-17T08:43:06Z

Rebased, let's go again!

@bors: r+ p=11

bors · 2018-04-17T08:43:07Z

📌 Commit 05275da has been approved by GuillaumeGomez

bors · 2018-04-17T09:02:09Z

⌛ Testing commit 05275da with merge 8728c7a...

@QuietMisdreavus

…GuillaumeGomez Add warning if a resolution failed r? @QuietMisdreavus

bors · 2018-04-17T11:31:14Z

☀️ Test successful - status-appveyor, status-travis
Approved by: GuillaumeGomez
Pushing 8728c7a to master...

rust-highfive assigned QuietMisdreavus Mar 31, 2018

GuillaumeGomez force-pushed the intra-link-resolution-error branch 2 times, most recently from e507a56 to dc4a9eaCompare April 1, 2018 19:11

Mark-Simulacrum reviewed Apr 1, 2018
View reviewed changes

GuillaumeGomez force-pushed the intra-link-resolution-error branch from dc4a9ea to fb1aae2Compare April 1, 2018 20:20

oli-obk reviewed Apr 1, 2018
View reviewed changes

GuillaumeGomez force-pushed the intra-link-resolution-error branch from fb1aae2 to efadab3Compare April 2, 2018 15:39

emilyalbini added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 2, 2018

QuietMisdreavus reviewed Apr 2, 2018
View reviewed changes

GuillaumeGomez force-pushed the intra-link-resolution-error branch from efadab3 to 1833b19Compare April 2, 2018 22:15

nrc mentioned this pull request Apr 3, 2018
Tracking issue for RFC 1946 - intra-rustdoc links #43466
Closed
27 tasks

Mark-Simulacrum reviewed Apr 5, 2018
View reviewed changes

GuillaumeGomez force-pushed the intra-link-resolution-error branch from 1833b19 to 46f62baCompare April 5, 2018 22:37

oli-obk reviewed Apr 6, 2018
View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 14, 2018

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 14, 2018

bors added a commit that referenced this pull request Apr 15, 2018
Auto merge of#49542- GuillaumeGomez:intra-link-resolution-error, r=…
f6d0bfd
…Mark-Simulacrum Add warning if a resolution failed r? @QuietMisdreavus

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 15, 2018

kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 16, 2018

GuillaumeGomez added 6 commits April 16, 2018 23:33

Add warning if a resolution failed
035ec5b

Add error-format and color-config options to rustdoc
a6fefde

Add rustdoc-ui test suite
b2192ae

Fix nits
16a3938

Fix empty tests
a3ed2ab

Remove unwanted auto-linking and update
05275da

GuillaumeGomez force-pushed the intra-link-resolution-error branch from 4b7f4cc to 05275daCompare April 16, 2018 21:41

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 17, 2018

bors added a commit that referenced this pull request Apr 17, 2018
Auto merge of#49542- GuillaumeGomez:intra-link-resolution-error, r=…
8728c7a
…GuillaumeGomez Add warning if a resolution failed r? @QuietMisdreavus

bors merged commit 05275da into rust-lang:masterApr 17, 2018

bors mentioned this pull request Apr 17, 2018
Remove uses of Build across Builder steps #49972
Merged

GuillaumeGomez deleted the intra-link-resolution-error branch April 17, 2018 14:12

QuietMisdreavus mentioned this pull request Aug 3, 2018
Stabilize --color and --error-format options in rustdoc #53003
Merged

pnkfelix mentioned this pull request Dec 3, 2018
NLL error on closure, but not on equivalent function #55526
Closed

cuviper mentioned this pull request Nov 5, 2024
Fix an extra newline in rendered std doc #132617
Merged

Uh oh!

Uh oh!

Conversation

GuillaumeGomez commented Mar 31, 2018

Uh oh!

oli-obk commented Mar 31, 2018

Uh oh!

GuillaumeGomez commented Mar 31, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Apr 1, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Apr 2, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QuietMisdreavus commented Apr 2, 2018

Uh oh!

GuillaumeGomez commented Apr 4, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oli-obk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Apr 7, 2018

Uh oh!

GuillaumeGomez commented Apr 13, 2018

Uh oh!

Mark-Simulacrum commented Apr 14, 2018

Uh oh!

GuillaumeGomez commented Apr 14, 2018

Uh oh!