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-json: Add test for field ordering. #110435

Merged
merged 1 commit into from
Aug 10, 2023
Merged

rustdoc-json: Add test for field ordering. #110435

merged 1 commit into from
Aug 10, 2023

Conversation

aDotInTheVoid
Copy link
Member

Inspired by this on twitter, the ordering of fields really matters, so we should test we preserve it through json.

r? rustdoc

@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 Apr 16, 2023
@rust-cloud-vms rust-cloud-vms bot force-pushed the rdj-field-ordering branch from 61c7b1e to 9139cf1 Compare April 17, 2023 10:14
@GuillaumeGomez
Copy link
Member

I suppose it needs to be checked for unions and enum struct variants as well?

@rust-cloud-vms rust-cloud-vms bot force-pushed the rdj-field-ordering branch from 9139cf1 to 3242b4f Compare April 18, 2023 13:43
@obi1kenobi
Copy link
Member

This looks great!

If you're open to adding one more thing, it would be great to assert that the order of variants in an enum is preserved as well. There are some sneaky non-obvious semver hazards there:

cargo-semver-checks is probably going to support both of these checks before too long, and it would be awesome to be sure that rustdoc preserves variant order.

@rust-cloud-vms rust-cloud-vms bot force-pushed the rdj-field-ordering branch from c0e0f4d to 006903e Compare April 18, 2023 15:18
@GuillaumeGomez
Copy link
Member

Sorry, I missed this one. Don't hesitate to ping me if approval is taking too long. ^^'

Thanks!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 8, 2023

📌 Commit 006903e has been approved by GuillaumeGomez

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 Aug 8, 2023
@Dylan-DPC Dylan-DPC mentioned this pull request Aug 8, 2023
Closed
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 8, 2023
@Dylan-DPC
Rollup merge of rust-lang#110435 - aDotInTheVoid:rdj-field-ordering, …
806b1cd 
…r=GuillaumeGomez

rustdoc-json: Add test for field ordering.

Inspired by [this on twitter](https://twitter.com/PredragGruevski/status/1647705616650043392), the ordering of fields really matters, so we should test we preserve it through json.

r? rustdoc
@matthiaskrgr
Copy link
Member

Looks like this failed in #114627 (comment) ?
@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 8, 2023
@rust-cloud-vms rust-cloud-vms bot force-pushed the rdj-field-ordering branch from 006903e to 52a4e3b Compare August 8, 2023 19:09
@GuillaumeGomez
Copy link
Member

r=me once CI pass

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-cloud-vms rust-cloud-vms bot force-pushed the rdj-field-ordering branch from 52a4e3b to 31ddea6 Compare August 9, 2023 10:26
@aDotInTheVoid
Copy link
Member Author

@bors r=GuillaumeGomez

@bors
Copy link
Contributor

bors commented Aug 9, 2023

📌 Commit 31ddea6 has been approved by GuillaumeGomez

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 9, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Aug 9, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 9, 2023
@bors
Auto merge of rust-lang#114673 - matthiaskrgr:rollup-9kroqpp, r=matth…
832db2f 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#110435 (rustdoc-json: Add test for field ordering.)
 - rust-lang#111891 (feat: `riscv-interrupt-{m,s}` calling conventions)
 - rust-lang#114377 (test_get_dbpath_for_term(): handle non-utf8 paths (fix FIXME))
 - rust-lang#114469 (Detect method not found on arbitrary self type with different mutability)
 - rust-lang#114587 (Convert Const to Allocation in smir)
 - rust-lang#114670 (Don't use `type_of` to determine if item has intrinsic shim)

Failed merges:

 - rust-lang#114599 (Add impl trait declarations to SMIR)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4e7e2a5 into rust-lang:master Aug 10, 2023
@rustbot rustbot added this to the 1.73.0 milestone Aug 10, 2023
@aDotInTheVoid aDotInTheVoid deleted the rdj-field-ordering branch August 10, 2023 19:51
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Sep 6, 2023
@bors
Auto merge of rust-lang#114673 - matthiaskrgr:rollup-9kroqpp, r=matth…
0777ae6 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#110435 (rustdoc-json: Add test for field ordering.)
 - rust-lang#111891 (feat: `riscv-interrupt-{m,s}` calling conventions)
 - rust-lang#114377 (test_get_dbpath_for_term(): handle non-utf8 paths (fix FIXME))
 - rust-lang#114469 (Detect method not found on arbitrary self type with different mutability)
 - rust-lang#114587 (Convert Const to Allocation in smir)
 - rust-lang#114670 (Don't use `type_of` to determine if item has intrinsic shim)

Failed merges:

 - rust-lang#114599 (Add impl trait declarations to SMIR)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

@obi1kenobi obi1kenobi obi1kenobi approved these changes

Assignees

@GuillaumeGomez GuillaumeGomez

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.73.0
Development

Successfully merging this pull request may close these issues.
7 participants
@aDotInTheVoid @GuillaumeGomez @obi1kenobi @bors @matthiaskrgr @rust-log-analyzer @rustbot