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

Remove unused impl and unnecessary struct-wrapper around tuple #2290

Merged
merged 3 commits into from Jul 6, 2023
Merged

Remove unused impl and unnecessary struct-wrapper around tuple #2290

merged 3 commits into from Jul 6, 2023

Conversation

Mingun
Copy link
Contributor

@Mingun Mingun commented Oct 2, 2022 •
edited

This PR does some minor cleanups in the code:

  • Implementation of DeserializeSeed for TaggedContentVisitor is never used, so it is removed
  • struct TaggedContent is used only in one place in the generated code, so it will be replaced by usual tuple

@Mingun Mingun force-pushed the enum-tests-and-cleanup branch 3 times, most recently from 4e319dd to 0c79e98 Compare February 25, 2023 17:55
@Mingun Mingun changed the title Add a expand tests for adjacently, internally and untagged enum representations and remove unused code Remove unused impl and unnecessary struct-wrapper around tuple Apr 22, 2023
oli-obk
oli-obk requested changes Apr 23, 2023
View reviewed changes
serde/src/private/de.rs Show resolved Hide resolved
serde_derive/src/de.rs Show resolved Hide resolved
dtolnay
dtolnay requested changes May 5, 2023
View reviewed changes
serde_derive/src/de.rs Outdated Show resolved Hide resolved
@Mingun Mingun requested a review from dtolnay May 5, 2023 15:55
dtolnay
dtolnay requested changes May 5, 2023
View reviewed changes
serde_derive/src/de.rs Outdated Show resolved Hide resolved
@Mingun Mingun requested a review from dtolnay May 6, 2023 14:31
@Mingun
Generate one deserializer rather than in each arm
780a461
@Mingun
Remove implementation of DeserializeSeed for TaggedContentVisitor
b61ec84 
It is not used anywhere
@Mingun
Remove TaggedContent, replace it by a tuple
3783a30 
That type does not give any benefits so we can avoid that hidden public but no-API struct
dtolnay
dtolnay approved these changes Jul 6, 2023
View reviewed changes
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks!

@dtolnay dtolnay merged commit 92bfc8d into serde-rs:master Jul 6, 2023
19 checks passed
@Mingun Mingun deleted the enum-tests-and-cleanup branch July 7, 2023 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk requested changes

@dtolnay dtolnay dtolnay approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Mingun @oli-obk @dtolnay