Skip to content

Conversation

MaloJaffre
Copy link
Contributor

@MaloJaffre MaloJaffre commented Nov 26, 2017

Fixes #47025.
I don't know if x86_64-gnu is the right builder for this, but there seems to be time left on Travis.

Remaining problems blocking this PR:

See individual commits for more details.

@rust-highfive
Copy link
Contributor

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Nov 26, 2017
@MaloJaffre
Copy link
Contributor Author

MaloJaffre commented Nov 26, 2017

Currently failing (probably #46271): https://travis-ci.org/rust-lang/rust/jobs/307535449#L6794-L6818.
At least that shows that the compiler docs are properly tested with this PR.

@Michael-F-Bryan
Copy link

@MaloJaffre, is this part of an overall fix for #29893?

@MaloJaffre
Copy link
Contributor Author

MaloJaffre commented Nov 28, 2017

@Michael-F-Bryan No, because I haven't read this issue before opening this PR, but it is a step forward, because, in the meantime, we can catch regressions of compiler docs building.

@kennytm kennytm added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 1, 2017
@kennytm
Copy link
Member

kennytm commented Dec 1, 2017

The fix for issue #46271 is PR #46405. Marking as S-blocked until that PR is merged.

@MaloJaffre
Copy link
Contributor Author

MaloJaffre commented Dec 3, 2017

Restarting CI to see if it passes, now that #46405 has been merged.

@MaloJaffre MaloJaffre closed this Dec 3, 2017
@MaloJaffre MaloJaffre reopened this Dec 3, 2017
@kennytm 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-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Dec 3, 2017
@MaloJaffre
Copy link
Contributor Author

MaloJaffre commented Dec 3, 2017

Now it's a lot of broken links in syntax, syntax_pos and rustc that failed the build!

EDIT: it seems that these are intended to be links to std docs, but they reference their own crates instead.

@carols10cents
Copy link
Member

Friendly ping to keep this on your radar, @MaloJaffre! How's it going?

@MaloJaffre
Copy link
Contributor Author

@carols10cents I'm currently trying to understand why the links are broken, but it's not very successful...

@steveklabnik
Copy link
Contributor

It depends per link; some of these point to a README.md, for example, which isn't actually rendered. Those links should be removed. Some of them might be broken due to re-exporting things, that is, if a link is put into something that's re-exported, it often breaks the link in one place since you can't write two different links for the same thing. You have to take it on a case-by-case basis.

@MaloJaffre
Copy link
Contributor Author

MaloJaffre commented Dec 13, 2017

After further investigation, this PR is blocked by: (EDIT: see PR description)
- rust-random/rand#207 and release of rand 0.3.19
- issue #32129 (syntax::symbol::InternedString derefs to &str)

Meanwhile, I will fix the other broken links that are really in compiler docs.

@kennytm kennytm added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Dec 13, 2017
@steveklabnik
Copy link
Contributor

The solution to the second one, for now, is just to remove the link.

@MaloJaffre
Copy link
Contributor Author

MaloJaffre commented Dec 15, 2017

@steveklabnik Did you mean to add doc(hidden) to the Deref impl for InternedString and others? I'm not sure it's a good idea to remove the links in &str docs which are broken when items implement Deref<Target=str>.

@MaloJaffre MaloJaffre changed the title [WIP] Add compiler docs testing to CI. Add compiler docs testing to CI. Dec 31, 2017
@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Dec 31, 2017
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps link the README to https://github.com/rust-lang/rust/blob/master/src/librustc/dep_graph/README.md instead of removing the link?

(Alternatively, make use of #![doc(include="README.md")] in mod.rs to display the README directly in the rustdoc. This could be another PR once we get the compiler docs into the CI.)

Copy link
Member

Choose a reason for hiding this comment

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

This block should be ```text?

Copy link
Member

Choose a reason for hiding this comment

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

Better merge this block with the one above and make it ```text.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps `…` it instead of \ it.

Copy link
Member

Choose a reason for hiding this comment

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

Ditto

Copy link
Member

Choose a reason for hiding this comment

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

Ditto

Copy link
Member

Choose a reason for hiding this comment

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

Please backquote the path.

@MaloJaffre
Copy link
Contributor Author

MaloJaffre commented Dec 31, 2017

@kennytm Thanks for your review!
I've fixed everything according to your remarks, except for the list formatting (I wasn't sure exactly how to format this part).

EDIT: After checking formatting of other lists in API docs, I applied your suggested formatting.

@bors
Copy link
Collaborator

bors commented Jan 1, 2018

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

It tested rust-lang#44953.
`log` macros in newer versions are no longer recursive, so these duplicated
error messages (about unstable feature uses) previously occurring at each
level of recursion are no longer possible, even with the fix by rust-lang#45540.
Furthermore this test breaks when multiple versions of `log` are in the
sysroot (`log 0.3.9` depends on`log 0.4.1`)
Update `rand` crate to `0.3.19`.
Update `log` crate to `0.3.9` and `0.4.1`.
Update `parking_lot_core` crate to `0.2.9`.
Upgrade all flate2 dependencies to `1.0.1`.
- Update `rust-installer` submodule.
@kennytm
Copy link
Member

kennytm commented Jan 1, 2018

Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 1, 2018

📌 Commit 2449230 has been approved by kennytm

bors added a commit that referenced this pull request Jan 1, 2018
Add compiler docs testing to CI.

Fixes #47025.
I don't know if `x86_64-gnu` is the right builder for this, but there seems to be time left on [Travis](https://travis-ci.org/rust-lang/rust/jobs/307488864).

Remaining problems blocking this PR:
- [x] broken links caused by rustdoc issues:
  - [x] `pub use self::Enum::...`: #46766 and #46767 (fixed by #47050, thanks @ollie27!)
  - [x] `impl Deref for DerefToStdType`: #32129 (ignored in linkchecker)
  - [x] `#[feature(decl_macro)]` and `use std::vec`: #47038 (ignored in linkchecker)
  - [x]  `rustc_data_structures::sync::{Lrc, RwLock}` aliases `std` types: #32130 (ignored in linkchecker)
- [x] markdown differences, in rust repository and in external crates, now failing the build with #46880 merged (all fixed)
- [x] multiple crate updates needed: `rand`, `log`, `parking_lot_core`, `flate2`
  - [x] submodule updates needed to deduplicate dependencies: `rust-installer`, ~`cargo`~ (done by #47052)
  - [x] #44953 test broken by `log` update (removed, this can be controversial)
- [x] Waiting `x86_64-gnu` build results ([done](https://travis-ci.org/rust-lang/rust/builds/323451069))

See individual commits for more details.
@bors
Copy link
Collaborator

bors commented Jan 1, 2018

⌛ Testing commit 2449230 with merge 5deba22...

@bors
Copy link
Collaborator

bors commented Jan 1, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: kennytm
Pushing 5deba22 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test internal docs on CI

10 participants