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

Fix jump def background #88885

Merged
merged 2 commits into from
Sep 14, 2021
Merged

Conversation

GuillaumeGomez
Copy link
Member

Fixes #88870.

I somehow badly wrote the color in #88111.

r? @camelid

@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 12, 2021
@GuillaumeGomez GuillaumeGomez added A-rustdoc-ui Area: Rustdoc UI (generated HTML) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 12, 2021
Copy link
Member

@camelid camelid left a comment

Choose a reason for hiding this comment

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

By the way, one of the GUI tests seems to be failing.

src/bootstrap/test.rs Outdated Show resolved Hide resolved
src/test/rustdoc-gui/code-sidebar-toggle.goml Outdated Show resolved Hide resolved
@camelid camelid added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 12, 2021
src/bootstrap/test.rs Outdated Show resolved Hide resolved
src/test/rustdoc-gui/code-sidebar-toggle.goml Outdated Show resolved Hide resolved
@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Sep 12, 2021

But why did you make the change in this PR? Could you open a separate PR for this change since it seems to be unrelated?

I have put it in a commit on its own but let's make another PR for it.

EDIT: PR is #88896

@GuillaumeGomez
Copy link
Member Author

Updated!

@GuillaumeGomez GuillaumeGomez added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 12, 2021
@GuillaumeGomez
Copy link
Member Author

Updated!

Copy link
Member

@camelid camelid left a comment

Choose a reason for hiding this comment

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

r=me after addressing these comments and squashing the second and third commits together (it's confusing to have them be separate)

src/bootstrap/test.rs Show resolved Hide resolved
src/test/rustdoc-gui/src/link_to_definition/lib.rs Outdated Show resolved Hide resolved
@camelid camelid 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 Sep 12, 2021
@GuillaumeGomez
Copy link
Member Author

Updated!

@GuillaumeGomez GuillaumeGomez added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 13, 2021
@camelid
Copy link
Member

camelid commented Sep 13, 2021

Could you squash the second and third commits together? It's confusing to have them be separate since neither can stand alone.

r=me after that

@camelid camelid removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 13, 2021
@GuillaumeGomez
Copy link
Member Author

@bors: r=camelid rollup

@bors
Copy link
Contributor

bors commented Sep 13, 2021

📌 Commit 9e482c1 has been approved by camelid

@bors 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 Sep 13, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 13, 2021
…und, r=camelid

Fix jump def background

Fixes rust-lang#88870.

I somehow badly wrote the color in rust-lang#88111.

r? `@camelid`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 13, 2021
Reduce possibility of flaky tests

As asked in rust-lang#88885.

r? `@camelid`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 13, 2021
Reduce possibility of flaky tests

As asked in rust-lang#88885.

r? ``@camelid``
@camelid
Copy link
Member

camelid commented Sep 13, 2021

I always keep the tests in a different commit but I can merge the second and third together.

The reason I think it's confusing is because in the first commit you were enabling --generate-link-to-definition on the test before the test even existed. Even if the commits were reversed, the commit with the test would fail because --generate-link-to-definition wouldn't be passed.

@GuillaumeGomez
Copy link
Member Author

Good point. :)

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Sep 14, 2021
…und, r=camelid

Fix jump def background

Fixes rust-lang#88870.

I somehow badly wrote the color in rust-lang#88111.

r? `@camelid`
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Sep 14, 2021
Reduce possibility of flaky tests

As asked in rust-lang#88885.

r? ```@camelid```
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Sep 14, 2021
…und, r=camelid

Fix jump def background

Fixes rust-lang#88870.

I somehow badly wrote the color in rust-lang#88111.

r? ``@camelid``
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Sep 14, 2021
Reduce possibility of flaky tests

As asked in rust-lang#88885.

r? ````@camelid````
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 14, 2021
…laumeGomez

Rollup of 7 pull requests

Successful merges:

 - rust-lang#88033 (Add links for primitives in "jump to definition" feature)
 - rust-lang#88722 (Make `UnsafeCell::get_mut` const)
 - rust-lang#88851 (Fix duplicate bounds for const_trait_impl)
 - rust-lang#88859 (interpreter PointerArithmetic: use new Size helper methods)
 - rust-lang#88885 (Fix jump def background)
 - rust-lang#88894 (Improve error message for missing trait in trait impl)
 - rust-lang#88896 (Reduce possibility of flaky tests)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f10fc21 into rust-lang:master Sep 14, 2021
@rustbot rustbot added this to the 1.57.0 milestone Sep 14, 2021
@GuillaumeGomez GuillaumeGomez deleted the fix-jump-def-background branch September 14, 2021 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustdoc (unstable): --generate-link-to-definition makes sources unreadable with Ayu theme
5 participants