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: Resolve conflict between "debugId" and "debug_id" #100

Merged
merged 4 commits into from
Nov 26, 2024

Conversation

loewenheim
Copy link
Contributor

Using an alias produces an error when both fields are set. The intended behavior is to only read from "debug_id" if "debugId" is not present. We now encode this logic explicitly by deserializing both and choosing manually.

@loewenheim loewenheim requested review from lforst and a team November 26, 2024 09:25
@loewenheim loewenheim self-assigned this Nov 26, 2024
src/jsontypes.rs Outdated
pub debug_id: Option<DebugId>,
// This field only exists to be able to deserialize from "debug_id" keys
// if "debugId" is unset.
#[serde(skip_serializing, rename = "debug_id")]
Copy link
Member

Choose a reason for hiding this comment

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

This may be a problem when deserializing and immediately serializing again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. skip_serializing_if should take care of that.

@loewenheim loewenheim merged commit c223b37 into master Nov 26, 2024
6 checks passed
@loewenheim loewenheim deleted the fix/debug-id-alias branch November 26, 2024 09:40
@loewenheim loewenheim mentioned this pull request Nov 26, 2024
loewenheim added a commit that referenced this pull request Nov 26, 2024
#100 overzealously switched the default key from debug_id to debugId. While both should be supported, we should default to the current debug_id for now and make the switch in a considered manner.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants