Remove is_spotlight field from `Trait` by GuillaumeGomez · Pull Request #80914 · rust-lang/rust

GuillaumeGomez · 2021-01-11T14:25:03Z

Small PR, only the last commit is relevant here. The rest is coming from #80883 because I need the TyCtxt stored inside Cache.

The point is to make ItemKind looks as close as possible to the compiler type so that it makes the switch simpler (which is why I make all these "small" PRs).

r? @jyn514

src/librustdoc/clean/utils.rs

jyn514 · 2021-01-12T03:25:01Z

@camelid would you like to review?

camelid · 2021-01-12T03:43:44Z

@jyn514 Thanks for giving me the opportunity to review! :)

I'll give it a look, but it's probably still a good idea for you to take a look as well since I'm still learning the intricacies of rustdoc.

camelid · 2021-01-12T03:44:37Z

(Marking as blocked on #80883.)

camelid

Is this field being removed because it's only used in one place and we don't want to pile stuff onto clean::Trait?

src/librustdoc/clean/inline.rs

src/librustdoc/clean/utils.rs

src/librustdoc/html/render/mod.rs

bors · 2021-01-21T16:24:10Z

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

…n514 Remove CACHE_KEY global We realized in rust-lang#80914 that the cache handling (through a global) needed to be updated to make it much easier to handle. r? `@jyn514`

GuillaumeGomez · 2021-01-28T16:06:27Z

Rebased! Only the last commit is from this PR, the others still come from #80883.

cc @CraftSpider @ollie27
r? @camelid

CraftSpider

Looks good. As far as I can tell this should behave the same, just with spotlight calculated at use instead of early. Only question is: How is spotlight_decl called, will this end up duplicating the spotlight lookup? Not too terrible if it does, but want to make sure I understand.

GuillaumeGomez · 2021-01-28T16:49:12Z

That's actually a pretty good point. It forces us to run this check every time a function returns a "spotlighted" item. That's not great.

* Improve documentation * Add a test to ensure that spotlighted traits from dependencies are taken into account as expected

GuillaumeGomez · 2021-02-23T21:14:45Z

r=me with nits fixed and the description updated - I don't think this needs to mention #80883 now that it's been merged.

I updated the description but I'll keep the mention to the other PR. ;)

GuillaumeGomez · 2021-02-23T22:09:56Z

@bors: r=jyn514

bors · 2021-02-23T22:09:58Z

📌 Commit 33aaead has been approved by jyn514

bors · 2021-02-24T10:05:06Z

⌛ Testing commit 33aaead with merge f46c0e49c6121d2ea31376c25c1ac1b7b91d0c91...

bors · 2021-02-24T14:20:06Z

💥 Test timed out

GuillaumeGomez · 2021-02-24T14:20:59Z

@bors: retry

bors · 2021-02-24T14:46:26Z

⌛ Testing commit 33aaead with merge a8486b6...

bors · 2021-02-24T17:48:38Z

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing a8486b6 to master...

I didn't make these renames in rust-lang#80965 because I didn't want the PR to conflict with rust-lang#80914.

…ht, r=GuillaumeGomez rustdoc: Rename internal uses of `spotlight` I didn't make these renames in rust-lang#80965 because I didn't want the PR to conflict with rust-lang#80914.

rust-highfive assigned jyn514 Jan 11, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 11, 2021

GuillaumeGomez force-pushed the remove-is_spotlight branch from d20dfd5 to 19d6700 Compare January 11, 2021 14:25

LingMan reviewed Jan 11, 2021

View reviewed changes

src/librustdoc/clean/utils.rs Outdated Show resolved Hide resolved

GuillaumeGomez force-pushed the remove-is_spotlight branch 2 times, most recently from 7254d97 to 2ef594d Compare January 11, 2021 21:34

camelid added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jan 12, 2021

camelid added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Jan 12, 2021

camelid reviewed Jan 12, 2021

View reviewed changes

src/librustdoc/clean/inline.rs Outdated Show resolved Hide resolved

src/librustdoc/clean/utils.rs Outdated Show resolved Hide resolved

src/librustdoc/html/render/mod.rs Outdated Show resolved Hide resolved

This was referenced Jan 12, 2021

Rename doc(spotlight) to doc(notable_trait) #80936

Closed

Rename #[doc(spotlight)] to #[doc(notable_trait)] #80965

Merged

GuillaumeGomez mentioned this pull request Jan 13, 2021

Remove CACHE_KEY global #80987

Merged

GuillaumeGomez force-pushed the remove-is_spotlight branch from 2ef594d to e86f706 Compare January 28, 2021 16:05

rust-highfive assigned camelid and unassigned jyn514 Jan 28, 2021

This comment has been minimized.

Sign in to view

CraftSpider reviewed Jan 28, 2021

View reviewed changes

GuillaumeGomez changed the title ~~Remove is spotlight field from Trait~~ Remove is_spotlight field from Trait Jan 28, 2021

GuillaumeGomez added 2 commits February 23, 2021 21:58

* Fix some typo

3c59e64

* Improve documentation * Add a test to ensure that spotlighted traits from dependencies are taken into account as expected

Improve code readability

33aaead

GuillaumeGomez force-pushed the remove-is_spotlight branch from 1593598 to 33aaead Compare February 23, 2021 21:14

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 Feb 23, 2021

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 Feb 24, 2021

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 Feb 24, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 24, 2021

bors merged commit a8486b6 into rust-lang:master Feb 24, 2021

rustbot added this to the 1.52.0 milestone Feb 24, 2021

This was referenced Feb 24, 2021

Make cfg imply doc(cfg) #79341

Closed

Remove the dummy cache in DocContext; delete RenderInfo #82018

Merged

Remove RefCell around module_trait_cache #82402

Merged

GuillaumeGomez deleted the remove-is_spotlight branch February 24, 2021 18:15

camelid mentioned this pull request Apr 1, 2021

rustdoc: Rename internal uses of spotlight #83756

Merged

camelid added a commit to camelid/rust that referenced this pull request Apr 2, 2021

rustdoc: Rename internal uses of spotlight

5f96c48

I didn't make these renames in rust-lang#80965 because I didn't want the PR to conflict with rust-lang#80914.

Uh oh!

Conversation

GuillaumeGomez commented Jan 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jyn514 commented Jan 12, 2021

Uh oh!

camelid commented Jan 12, 2021

Uh oh!

camelid commented Jan 12, 2021

Uh oh!

camelid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bors commented Jan 21, 2021

Uh oh!

GuillaumeGomez commented Jan 28, 2021

Uh oh!

This comment has been minimized.

CraftSpider left a comment

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Jan 28, 2021

Uh oh!

GuillaumeGomez commented Feb 23, 2021

Uh oh!

GuillaumeGomez commented Feb 23, 2021

Uh oh!

bors commented Feb 23, 2021

Uh oh!

bors commented Feb 24, 2021

Uh oh!

bors commented Feb 24, 2021

Uh oh!

GuillaumeGomez commented Feb 24, 2021

Uh oh!

bors commented Feb 24, 2021

Uh oh!

bors commented Feb 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

GuillaumeGomez commented Jan 11, 2021 •

edited

Loading