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

fixes: #362 Locale problem with JsonbNumberFormat #363

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

struberg
Copy link

No description provided.

@@ -20,6 +20,7 @@
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.util.Locale;

This comment was marked as resolved.

Copy link

@njr-11 njr-11 left a comment

Choose a reason for hiding this comment

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

I think you also need a 2024 copyright on this file.

@@ -36,6 +37,11 @@
* <li> package </li>
* </ul>
*
* <p>Note that even when specifying a pattern format your JSON is depending on the Locale of the JVM you run on.
Copy link

Choose a reason for hiding this comment

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

This sentence is difficult to understand without punctuation to identify which of the different parts go together and which don't. Here is one suggestion to clarify it while leaving it mostly as written,

Suggested change
* <p>Note that even when specifying a pattern format your JSON is depending on the Locale of the JVM you run on.
* <p>Note that even when specifying a pattern, the format of the JSON value depends on the Locale of the JVM by default.

Copy link
Author

Choose a reason for hiding this comment

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

txs for the review and suggestion!

@struberg
Copy link
Author

will update, txs for the feedback!

improvements in wording, copyright updated and unnecessary import removed.
@struberg struberg requested a review from njr-11 October 29, 2024 16:33
@Verdent Verdent linked an issue Nov 13, 2024 that may be closed by this pull request
@mkarg
Copy link
Contributor

mkarg commented Dec 23, 2024

@jakartaee/ee4j-jsonb-project-leads This PR was approved by two committers weeks many weeks ago, so I wonder why not merging it?

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.

clarify that @JsonbNumberFormat is depending on the Locale
5 participants