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

rustdoc-json-types: Use different repr for Constant in Item and Type position #125974

Open
aDotInTheVoid opened this issue Jun 4, 2024 · 3 comments
Assignees
Labels
A-rustdoc-json Area: Rustdoc JSON backend T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@aDotInTheVoid
Copy link
Member

aDotInTheVoid commented Jun 4, 2024

This is fine for now, but once this lands, I'd like to look into using seperate type for Item::Constant and Term::Constant/GenericArgKind::Constant, so this can be flattened again. But this shoudn't block this PR.

Originally posted by @aDotInTheVoid in #125958 (comment)

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 4, 2024
@aDotInTheVoid aDotInTheVoid self-assigned this Jun 4, 2024
@aDotInTheVoid aDotInTheVoid added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-json Area: Rustdoc JSON backend and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jun 4, 2024
@aDotInTheVoid
Copy link
Member Author

(Moving this discussion here because the linked PR is high-traffic, and I don't want to spam T-Types with this, and I want to unsubscribe from T-Types review)

As cargo-semver-checks maintainer, I'd love it if it's possible to make all the proposed representation changes quickly so that the separated representation is not the one that makes it into the next beta in ~9 days time.

If it goes into beta and then becomes stable, then I'll have to support both the separated representation and the newer form, and cargo-semver-checks will have to compile one more crate than otherwise — both negatives on my end.

If it's not possible to do that, and in your opinion it's better for the overall Rust community if I draw the short straw here, that's okay — I can handle it. I just would like to ask that it be a considered, intentional decision and not just unfortunate happenstance.

Originally posted by @obi1kenobi in #125958 (comment)

@aDotInTheVoid
Copy link
Member Author

is not the one that makes it into the next beta in ~9 days time.

given the magnitude of the ty change, it's quite likely it doesn't make the next beta split, but that's kinda beside the point.

If it goes into beta and then becomes stable

rustdoc-json is an nightly-only unstable feature that isn't supported on stable or beta. cargo-semver-checks deliberately bypasses the stability guarantees. I've expressed thaughts on this in the past.

I just would like to ask that it be a considered, intentional decision and not just unfortunate happenstance.

i don't know if i or anyone else is going to have either the time or the motivation to work on this soon. if this is important to you, you're welcome to work on this yourself (although given that the goal of this was to save you work, i'm not sure that's the best use of anyone's time)

@obi1kenobi
Copy link
Member

No worries. It sounds like the best outcome overall is if I just support the extra format in cargo-semver-checks. I asked for a considered, intentional decision and this definitely meets that bar — so I appreciate it even though it wasn't necessarily what I was hoping for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-json Area: Rustdoc JSON backend T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants