Skip to content

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

Merged
bors merged 1 commit intorust-lang:masterfrom
aDotInTheVoid:rdj-field-ordering
Aug 10, 2023
Merged

rustdoc-json: Add test for field ordering.#110435
bors merged 1 commit intorust-lang:masterfrom
aDotInTheVoid:rdj-field-ordering

Conversation

@aDotInTheVoid
Copy link
Member

@aDotInTheVoid aDotInTheVoid commented Apr 16, 2023

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
obi1kenobi
obi1kenobi reviewed Apr 16, 2023
View reviewed changes
@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

GuillaumeGomez commented Apr 17, 2023

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

obi1kenobi commented Apr 18, 2023

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

GuillaumeGomez commented Aug 8, 2023

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

Thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

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

matthiaskrgr commented Aug 8, 2023

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

GuillaumeGomez commented Aug 8, 2023

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

aDotInTheVoid commented Aug 9, 2023

@bors r=GuillaumeGomez

@bors
Copy link
Collaborator

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

+1 more reviewer

@obi1kenobi obi1kenobi obi1kenobi approved these changes

Reviewers whose approvals may not affect merge requirements

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