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

Fix NotNull locales #10216

Merged
merged 1 commit into from
Feb 9, 2024
Merged

Conversation

Moulberry
Copy link
Contributor

Two methods returning locales are annotated @NotNull, despite being able to return null

@Moulberry Moulberry requested a review from a team as a code owner February 3, 2024 13:39
@imDaniX
Copy link

imDaniX commented Feb 3, 2024

Is there a reason why the default fallback is the US locale? I'd say if player's locale cannot be parsed, it'd better be something like Locale.ROOT, so we can check for that and act accordingly, like warning a player.

@olijeffers0n
Copy link
Member

Is there a reason why the default fallback is the US locale? I'd say if player's locale cannot be parsed, it'd better be something like Locale.ROOT, so we can check for that and act accordingly, like warning a player.

Personally, I would say it is very standard to default to English. I am biased, but defaulting to the server's root Locale seems like it could cause issues

@imDaniX
Copy link

imDaniX commented Feb 3, 2024

Personally, I would say it is very standard to default to English. I am biased, but defaulting to the server's root Locale seems like it could cause issues

Well, there's also Locale.ENGLISH which is used fairly often too, hence the question. Also, the root locale isn't "server's root locale", but rather a neutral one, as stated in its javadoc.

@Moulberry
Copy link
Contributor Author

Is there a reason why the default fallback is the US locale? I'd say if player's locale cannot be parsed, it'd better be something like Locale.ROOT, so we can check for that and act accordingly, like warning a player.

Locale.US is the fallback locale used elsewhere:

+ public java.util.Locale adventure$locale = java.util.Locale.US; // Paper

I don't mind Locale.ROOT, but consistency with the existing code is more important

Two methods returning locales are annotated @NotNull, despite being able
to return null
@electronicboy electronicboy merged commit 534659e into PaperMC:master Feb 9, 2024
3 checks passed
lynxplay pushed a commit to lynxplay/paper that referenced this pull request Feb 23, 2024
Two methods returning locales are annotated @NotNull, despite being able
to return null
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

5 participants