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.

Sign up for GitHub

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

Jump to bottom

[rustdoc] Add is_object_safe information for traits in JSON output #119246

Merged
merged 3 commits into from
Dec 23, 2023
Merged

[rustdoc] Add is_object_safe information for traits in JSON output #119246

merged 3 commits into from
Dec 23, 2023

Conversation

GuillaumeGomez
Copy link
Member

@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Dec 23, 2023
@rustbot
Copy link
Collaborator

rustbot commented Dec 23, 2023

rustdoc-json-types is a public (although nightly-only) API. If possible, consider changing src/librustdoc/json/conversions.rs; otherwise, make sure you bump the FORMAT_VERSION constant.

cc @CraftSpider, @aDotInTheVoid, @Enselic, @obi1kenobi

aDotInTheVoid
aDotInTheVoid requested changes Dec 23, 2023
View reviewed changes
Copy link
Member

@aDotInTheVoid aDotInTheVoid left a comment

Choose a reason for hiding this comment

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

r? @aDotInTheVoid

r=me with assertions fixed

@rustbot rustbot assigned aDotInTheVoid and unassigned notriddle Dec 23, 2023
aDotInTheVoid
aDotInTheVoid reviewed Dec 23, 2023
View reviewed changes
tests/rustdoc-json/traits/is_object_safe.rs Outdated
#![no_std]

// @has "$.index[*][?(@.name=='FooUnsafe')]"
// @has "$.index[*][?(@.name=='FooUnsafe')].inner.trait.is_object_safe" false
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// @has "$.index[*][?(@.name=='FooUnsafe')].inner.trait.is_object_safe" false
// @is "$.index[*][?(@.name=='FooUnsafe')].inner.trait.is_object_safe" false

That way you get a better error message from jsondocck, (and also means this test will fail if this ever somehow mathces to [true, false]).

The top assert is redundant, but it's fine to keep or remove at your discression,

@notriddle
Copy link
Contributor

Please don’t r? me for JSON changes. I’m not familiar enough with that part of the code base.

@GuillaumeGomez GuillaumeGomez force-pushed the trait-is_object_safe-json branch from 53034ea to 431ac40 Compare December 23, 2023 18:00
@GuillaumeGomez
Copy link
Member Author

@notriddle Noted!

@bors r=aDotInTheVoid rollup

@bors
Copy link
Contributor

bors commented Dec 23, 2023

📌 Commit 431ac40 has been approved by aDotInTheVoid

It is now in the queue for this repository.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 23, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 23, 2023
Merged
@aDotInTheVoid
Copy link
Member

I’m not on review rotation, but am happy to review rustdoc-json stuff.

(is their a way to tell high-five to only assign me these PR’s, or should i rely on the ping, and then r ? myself?)

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 23, 2023
@bors
Auto merge of rust-lang#119263 - matthiaskrgr:rollup-zxok9fb, r=matth…
2d7be73 
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#112936 (Add illumos aarch64 target for rust.)
 - rust-lang#119153 (stabilize `file_create_new`)
 - rust-lang#119246 ([rustdoc] Add `is_object_safe` information for traits in JSON output)
 - rust-lang#119254 (Remove an unused diagnostic struct)
 - rust-lang#119255 (add a test for ICE rust-lang#112822)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 81161a6 into rust-lang:master Dec 23, 2023
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Dec 23, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 23, 2023
@rust-timer
Unrolled build for rust-lang#119246
e9b5a82 
Rollup merge of rust-lang#119246 - GuillaumeGomez:trait-is_object_safe-json, r=aDotInTheVoid

[rustdoc] Add `is_object_safe` information for traits in JSON output

As asked by `@obi1kenobi` [here](rust-lang#113241 (comment)).

cc `@aDotInTheVoid`
r? `@notriddle`
@GuillaumeGomez GuillaumeGomez deleted the trait-is_object_safe-json branch December 24, 2023 10:51
@GuillaumeGomez
Copy link
Member Author

There is a way, but since it's part of rustdoc, I'm not sure it's a good idea to have different handlings for subparts. In any case you're pinged normally so even though it's more of a hassle, you can self-assign when pinged.

@aDotInTheVoid aDotInTheVoid mentioned this pull request Dec 2, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aDotInTheVoid aDotInTheVoid aDotInTheVoid requested changes

Assignees

@aDotInTheVoid aDotInTheVoid

Labels
A-rustdoc-json Area: Rustdoc JSON backend 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
Milestone
1.77.0
Development

Successfully merging this pull request may close these issues.
5 participants
@GuillaumeGomez @rustbot @notriddle @bors @aDotInTheVoid