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

cleanup(serde_v8): streamline magic types #14076

Merged
merged 25 commits into from
Mar 23, 2022
Merged

cleanup(serde_v8): streamline magic types #14076

merged 25 commits into from
Mar 23, 2022

Conversation

AaronO
Copy link
Contributor

@AaronO AaronO commented Mar 22, 2022 •
edited
Loading

Formalize magic types and how they're "transerialized"

Also makes U16String deserializable

@AaronO
cleanup(serde_v8): streamline magic types
00f18a8 
Formalize magic types and how they're transerialized

Also makes `U16String` deserializable
lucacasonato
lucacasonato reviewed Mar 22, 2022
View reviewed changes
serde_v8/src/magic/tr8.rs Outdated Show resolved Hide resolved
serde_v8/src/magic/u16string.rs Show resolved Hide resolved
andreubotella
andreubotella reviewed Mar 22, 2022
View reviewed changes
serde_v8/src/de.rs Outdated Show resolved Hide resolved
serde_v8/src/magic/buffer.rs Outdated Show resolved Hide resolved
serde_v8/src/magic/buffer.rs Outdated Show resolved Hide resolved
serde_v8/src/magic/tr8.rs Outdated Show resolved Hide resolved
serde_v8/src/magic/tr8.rs Outdated Show resolved Hide resolved
aapoalas
aapoalas reviewed Mar 22, 2022
View reviewed changes
serde_v8/src/de.rs Outdated Show resolved Hide resolved
ry
ry reviewed Mar 23, 2022
View reviewed changes
serde_v8/src/magic/buffer.rs Outdated Show resolved Hide resolved
ry
ry reviewed Mar 23, 2022
View reviewed changes
serde_v8/src/ser.rs Outdated Show resolved Hide resolved
ry
ry reviewed Mar 23, 2022
View reviewed changes
Copy link
Member

@ry ry left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Would be good to have a unit test showing deserializing U16String

nevermind - i see it

ry
ry approved these changes Mar 23, 2022
View reviewed changes
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM - it's complex stuff that I don't understand completely, but tests pass.

@AaronO AaronO force-pushed the cleanup/serde-v8-transerializer branch from d65dfbf to dd4e0c8 Compare March 23, 2022 19:34
@AaronO AaronO merged commit f580134 into denoland:main Mar 23, 2022
@AaronO AaronO deleted the cleanup/serde-v8-transerializer branch March 23, 2022 20:15
kitsonk pushed a commit to kitsonk/deno that referenced this pull request Mar 23, 2022
@AaronO @kitsonk
cleanup(serde_v8): streamline magic types (denoland#14076)
6c39668 
Formalize magic types and how they're "transerialized", also makes U16String deserializable
kitsonk pushed a commit that referenced this pull request Mar 24, 2022
@AaronO @kitsonk
cleanup(serde_v8): streamline magic types (#14076)
e3c8c18 
Formalize magic types and how they're "transerialized", also makes U16String deserializable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andreubotella andreubotella andreubotella left review comments

@aapoalas aapoalas aapoalas left review comments

@littledivy littledivy littledivy left review comments

@ry ry ry approved these changes

@lucacasonato lucacasonato Awaiting requested review from lucacasonato

@bartlomieju bartlomieju Awaiting requested review from bartlomieju

@kt3k kt3k Awaiting requested review from kt3k

@piscisaureus piscisaureus Awaiting requested review from piscisaureus

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@AaronO @ry @lucacasonato @andreubotella @aapoalas @littledivy