Conversation
|
r? @jyn514 (rustbot has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
5d132ac to
eb84c41
Compare
|
@jyn514 This PR is ready. It is just the docs for those crates, not their dependencies, but I think the caching/freshness check is broken some[how,where]? |
|
☔ The latest upstream changes (presumably #106757) made this pull request unmergeable. Please resolve the merge conflicts. |
e9be87b to
bfe6f75
Compare
|
☔ The latest upstream changes (presumably #107143) made this pull request unmergeable. Please resolve the merge conflicts. |
bfe6f75 to
fbb159b
Compare
57dd4ab to
dfb40c3
Compare
jyn514
left a comment
There was a problem hiding this comment.
Can you see how long it takes to run tidy before and after this change? Please include the time it takes to run rustdoc -wjson on the standard library.
|
Sorry for the long delay btw; if you want to reassign this to Mark I'm ok with that. |
jyn514
left a comment
There was a problem hiding this comment.
this looks broadly right to me except for the stage issue :)
This comment was marked as outdated.
This comment was marked as outdated.
|
Oh right, I'd forgotten that you have a slow laptop 😓 Hmm, given that it's such a large slowdown, maybe we should move the check out of tidy? We can put it in the |
|
@jyn514 I have moved the generation of the pages to |
|
I don't think I'll have time to review this in the near future, unfortunately. r? bootstrap |
|
No problem, do what you think is the best. Welcome @ozkanonur to this PR! |
onur-ozkan
left a comment
There was a problem hiding this comment.
Greatk work! 🔥
I have some suggestions for this implementation:
There was a problem hiding this comment.
This can be defined outside of the loop.
Also, we can reduce the nested scopes with the following way
// Given a `NestedMeta` like `feature = "xyz"`, returns `xyz`.
let get_feature_name = |nested: &_| match nested {
NestedMeta::Meta(Meta::NameValue(name_value)) => {
if !is_ident(name_value.path.get_ident()?, "feature") {
return None;
}
match &name_value.lit {
Lit::Str(s) => Some(s.value()),
_ => None,
}
}
_ => None,
};There was a problem hiding this comment.
Is it only me that this looks hard to debug/understand the workflow inside? :)
Maybe we can have some comments to inform developer about what's going on in there. + Using expect instead of unwrap can be nice to have as well.
There was a problem hiding this comment.
This tries (quite badly) to generate the URL that the item belongs to.
If there is a way to get it directly from Rustdoc, I will happily use it.
Concerning the unwraps, this is mostly prototyping code that is not meant to last.
There was a problem hiding this comment.
This tries (quite badly) to generate the URL that the item belongs to.
If there is a way to get it directly from Rustdoc, I will happily use it.
Having comments which explains aim of the function instructions(which are complicated to read/track) can be handy imo.(e.g
Lines 1165 to 1930 in 91eb6f9
Concerning the unwraps, this is mostly prototyping code that is not meant to last.
It's just to give more clear information in context. Not so necessary I guess.
b133109 to
471a4f2
Compare
This comment was marked as resolved.
This comment was marked as resolved.
e055a87 to
2666686
Compare
|
☔ The latest upstream changes (presumably #113508) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@albertlarsan68 do we have any plans for this PR? |
|
The thing that I am having a hard time with is generating the links. The machinery to detect what is gated is in place, but the links are hard to generate. |
|
@albertlarsan68 any updates on this? |
|
Nothing new. |
Add the items to the autogenerated stubs Make unstable-book-gen use the top stage instead of stage 0 Generate the full docs, only print and save optional notes
2666686 to
ab4e806
Compare
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
☔ The latest upstream changes (presumably #120491) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@albertlarsan68 if you can rebase this and resolve the conflicts , we can push this forward for a review |
|
@albertlarsan68 @rustbot label: +S-inactive |
Helps with #103548