Skip to content

CLDR-18697 Use static PathStarrer.computeIfAbsent where possible #4807

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

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

Conversation

btangmu
Copy link
Member

@btangmu btangmu commented Jun 11, 2025

-Assume that it is safe to use STAR_PATTERN instead of simple *

-Change starrer.set to static PathStarrer.computeIfAbsent if pattern is * or unchanged from default STAR_PATTERN, unless getAttributes is called on an instance

-Remove implements Transform and the transform method, apparently unused for PathStarrer

-Remove unused method getSubstitutionPattern

-Remove already-unused ExampleCache.pathStarrer; revise related comments

-Rename the 2-argument set method to setSkippingAttributes, to disambiguate at least for now; the 2-argument method has only a single caller, in TestAlt

-In the 1-argument set method, avoid calling the 2-argument method, for simplicity and performance, at least for now

CLDR-18697

  • This PR completes the ticket.

ALLOW_MANY_COMMITS=true

-Assume that it is safe to use STAR_PATTERN instead of simple *

-Change starrer.set to static PathStarrer.computeIfAbsent if pattern is * or unchanged from default STAR_PATTERN, unless getAttributes is called on an instance

-Remove implements Transform and the transform method, apparently unused for PathStarrer

-Remove unused method getSubstitutionPattern

-Rename the 2-argument set method to setSkippingAttributes, to disambiguate at least for now; the 2-argument method has only a single caller, in TestAlt

-In the 1-argument set method, avoid calling the 2-argument method, for simplicity and performance, at least for now
@btangmu btangmu self-assigned this Jun 11, 2025
@@ -286,7 +285,7 @@ static void showInconsistenciesInLocale(String locale) {
continue;
}
if (SHOW_PROGRESS_RAW) {
ps.set(path);
PathStarrer.computeIfAbsent(path);
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 strange, looks like the value is never used.

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems like that was already the case. Also, SHOW_PROGRESS_RAW is false, so maybe this is part of an incomplete unused feature.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've removed that line in the 2nd commit

-In ShowInconsistentAvailable, the starred path was computed but unused

-Note that the code in question also depends on SHOW_PROGRESS_RAW, which is false
@btangmu btangmu requested a review from macchiati June 12, 2025 11:49
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.

2 participants