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

Migrate remaining text-serializer-gson serializers to use TypeAdapters #373

Merged
merged 22 commits into from
Sep 4, 2021
Merged

Migrate remaining text-serializer-gson serializers to use TypeAdapters #373

merged 22 commits into from
Sep 4, 2021

Conversation

emilyy-dev
Copy link
Contributor

@emilyy-dev emilyy-dev mentioned this pull request May 19, 2021
Closed
4 tasks
@emilyy-dev emilyy-dev requested a review from kashike May 20, 2021 03:13
@emilyy-dev emilyy-dev marked this pull request as ready for review May 21, 2021 04:31
@emilyy-dev emilyy-dev requested a review from kashike May 21, 2021 04:32
@emilyy-dev emilyy-dev marked this pull request as draft May 21, 2021 04:36
@emilyy-dev emilyy-dev marked this pull request as ready for review May 21, 2021 05:06
@emilyy-dev emilyy-dev requested a review from kashike June 1, 2021 20:54
@emilyy-dev
Convert ShowItemSerializer to TypeAdapter
eaf22a7
@emilyy-dev
Write kashike-compliant code
7faa7da
@emilyy-dev
Use a TypeAdapterFactory for better handling dependencies between ser…
48da0be 
…ializers in the future
@emilyy-dev
Write more kashike-compliant code
4349c4d
@emilyy-dev
Convert StyleSerializer to TypeAdapter
352b221
@emilyy-dev
Convert ShowEntitySerializer to TypeAdapter
6ea403e
@emilyy-dev
Remove unnecessary imports
d4d1f5d
@emilyy-dev
Move indexed serializers to static instances
60353a9
@emilyy-dev
Use static types in SerializerFactory when retrieving Gson adapters
e1d72d0
@emilyy-dev
Move indexed serializers to static instances once again into dummy cl…
0bfe5c2 
…asses
@emilyy-dev
Prefer direct references to serializers over Gson#getAdapter
4cef6bf 
Pseudo-migration of ComponentSerializerImpl to TypeAdapter, it's just a façade
@emilyy-dev
Prevent infinite recursion with hideous patterns
0978383
@emilyy-dev
Make the style checker happy
0223077
@emilyy-dev
Make the way of making the CSI & SES less hideous
6d71308
@emilyy-dev
Add spaces and use jetbrains annotations to comply with current check…
3db8bce 
…style

Signed-off-by: emilyy-dev <35617540+emilyy-dev@users.noreply.github.com>
@emilyy-dev
What's your favorite thing about space? Mine is space.
06ec1df 
Signed-off-by: emilyy-dev <35617540+emilyy-dev@users.noreply.github.com>
zml2008
zml2008 requested changes Aug 3, 2021
View reviewed changes
Copy link
Member

@zml2008 zml2008 left a comment

Choose a reason for hiding this comment

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

Thanks for doing this (and sorry for taking so long to get to reviewing)!

I'd rather you request TypeAdapter instances via a Gson instance in each adapter's constructor, rather than using static fields, at least where possible. That allows doing fun tricks with swapping out one adapter for another.

Second -- I've recently tossed in some wiring for JMH benchmarking. Would you be willing to set up some basic test cases for a before/after comparison? You should be able to use ComponentCompactionBenchmark in the api submodule as a reference for that.

...zer-gson/src/main/java/net/kyori/adventure/text/serializer/gson/ComponentSerializerImpl.java Outdated Show resolved Hide resolved
@emilyy-dev
Don't hard-code the serializer instances used throughout the serializers
066ec51 
Provide the active `Gson` instance and fetch its registered adapters

Signed-off-by: emilyy-dev <35617540+emilyy-dev@users.noreply.github.com>
@emilyy-dev
Add gson (de)serialization benchmarks
9f3edca 
Signed-off-by: emilyy-dev <35617540+emilyy-dev@users.noreply.github.com>
@emilyy-dev
Add license headers
cd8afb8 
Signed-off-by: emilyy-dev <35617540+emilyy-dev@users.noreply.github.com>
@zml2008 zml2008 added this to the 4.9.0 milestone Aug 15, 2021
@BomBardyGamer BomBardyGamer mentioned this pull request Aug 15, 2021
Closed
@emilyy-dev
Minor changes in de/serialization benchmarks
7b57876 
Signed-off-by: emilyy-dev <35617540+emilyy-dev@users.noreply.github.com>
@emilyy-dev emilyy-dev marked this pull request as draft August 24, 2021 18:27
@emilyy-dev
Actually migrate ComponentSerializerImpl to TypeAdapter
8cc2511 
Other misc changes

Signed-off-by: emilyy-dev <35617540+emilyy-dev@users.noreply.github.com>
@emilyy-dev emilyy-dev marked this pull request as ready for review September 3, 2021 20:12
zml2008
zml2008 approved these changes Sep 4, 2021
View reviewed changes
Copy link
Member

@zml2008 zml2008 left a comment

Choose a reason for hiding this comment

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

The ComponentSerializerImpl and StyleSerializer classes are definitely a touch messier than we'd like, but that's unavoidable I think. Thank you for pushing this through to the finish line!

@zml2008 zml2008 closed this in 0dde0b6 Sep 4, 2021
@zml2008 zml2008 merged commit 0dde0b6 into KyoriPowered:master Sep 4, 2021
@zml2008 zml2008 self-assigned this Sep 4, 2021
@zml2008 zml2008 linked an issue Sep 4, 2021 that may be closed by this pull request
Migrate remaining text-serializer-gson serializers to use TypeAdapters #372
Closed
4 tasks
This was referenced Sep 5, 2021
Merged
Merged
Closed
Closed
This was referenced Oct 31, 2021
Merged
Merged
Merged
Merged
Merged
This was referenced Dec 29, 2021
Closed
Merged
@renovate renovate bot mentioned this pull request Apr 13, 2022
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zml2008 zml2008 zml2008 approved these changes

@kashike kashike Awaiting requested review from kashike

Assignees

@zml2008 zml2008

Labels
area: text-serialiser/gson type: enhancement
Projects
None yet
Milestone
4.9.0
Development

Successfully merging this pull request may close these issues.

Migrate remaining text-serializer-gson serializers to use TypeAdapters
3 participants
@emilyy-dev @zml2008 @kashike