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

Clarify in documentation that Gson might cache strategy results #2215

Conversation

Marcono1234
Copy link
Collaborator

Resolves #2205

Maybe it could also be considered whether that behavior could be customized in the future, but for now it might suffice to document it.

Feedback, especially regarding the wording, is appreciated!

Copy link
Member

@eamonnmcmanus eamonnmcmanus left a comment

Choose a reason for hiding this comment

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

Seems good. Just one tiny comment (which applies several times).

@@ -349,6 +349,10 @@ public GsonBuilder setFieldNamingPolicy(FieldNamingPolicy namingConvention) {
* Configures Gson to apply a specific naming strategy to an object's fields during
* serialization and deserialization.
*
* <p>The Gson instance might only use the field naming strategy once for a field and
Copy link
Member

Choose a reason for hiding this comment

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

I think I would use {@link Gson} here to make it clear that we are talking about the class here (in the previous paragraph we are talking about Gson the project).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had a similar concern, being afraid that users might think Gson statically (for all Gson instances) caches the results. That is why I chose "Gson instance", but I have now changed it to "The created Gson instance". In the context of this builder I hope this should be clear enough.

In this specific line where you added the comment I don't think a {@link Gson} would be needed. The previous paragraph is also about the Gson instance and not about specifying a naming strategy globally. Or did I misunderstand you?

Copy link
Member

Choose a reason for hiding this comment

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

This is fine now. I would probably use {@link} or {@code} every time we're talking about Gson the class, since Gson is also the name of the project. But that's probably overly fussy.

@eamonnmcmanus eamonnmcmanus merged commit e614e71 into google:master Oct 4, 2022
@Marcono1234 Marcono1234 deleted the marcono1234/doc-cache-strategy-results branch October 4, 2022 18:47
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.

ExclusionStrategy cannot depend on threadlocal
2 participants