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

[Meta] Automatic String Organization #1372

Merged
merged 9 commits into from
Dec 21, 2024
Merged

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented Dec 20, 2024

Summary

Adds a step at the beginning of the build process that alphabetizes localizations.

The changes localizations in this commit were re-alphabetized by this new Swift Script. I think Swift and Python have different Alphabetization methods but if we're just using this by default, the Swift method should be used.

Finalizes #1362 and hopefully resolves #518

This script only comments & alphabetizes strings. This step does not validate whether a String is used in the project. I can add that step in as well if we want it but I do think there are some unused strings that are good to keep. A good example is Experimental since we're not using it now but may want to use it in the future.

If we want to remove unused strings, I can add it via another script if we like this script process!

@JPKribs JPKribs requested a review from LePips December 20, 2024 22:48
Scripts/AlphabetizeStrings.swift Outdated Show resolved Hide resolved
Copy link
Member

@LePips LePips left a comment

Choose a reason for hiding this comment

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

The automatic removal during the build phase isn't correct because it removes strings before they can even be used. I think I ideally want a job on GitHub that can detect unused strings and comment on PRs.

However, having the script around is still useful to run every once in a while.

For the scripts they don't need to run every time and only when en.lproj/Localizable.strings changes. However, even though I've followed the instructions at the section below the script still runs on every build. It's fine for now since these take < 0.5s on my machine.

@LePips LePips merged commit af602d3 into jellyfin:main Dec 21, 2024
4 checks passed
@JPKribs
Copy link
Member Author

JPKribs commented Dec 21, 2024

The automatic removal during the build phase isn't correct because it removes strings before they can even be used. I think I ideally want a job on GitHub that can detect unused strings and comment on PRs.

Sounds good! I probably wont take a crack at that one (at least for now) since Git is still a pretty weak skill for me. I'm fine with just the Alphabetizing for now since that should handle the primary duplicate entries + organization issues. Unused localized isn't that bad IMO since there are some items worth keep localized even if we aren't using them: (Eg, WIP, Experimental, etc.).

I'll let you make the call for whether or not this closes out #518!

@JPKribs JPKribs deleted the alphabetizeStrings branch December 21, 2024 08:07
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.

Clean up Localizations
2 participants