-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[produce,deliver,spaceship] Support for new AppStore Connect languages: Catalan, Croatian, Czech, Hindi, Hungarian, Polish, Romanian, Slovak, and Ukrainian #14110
[produce,deliver,spaceship] Support for new AppStore Connect languages: Catalan, Croatian, Czech, Hindi, Hungarian, Polish, Romanian, Slovak, and Ukrainian #14110
Conversation
* New languages: Catalan, Croatian, Czech, Hindi, Hungarian, Polish, Romanian, Slovak, and Ukrainian
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
I signed it! |
CLAs look good, thanks! |
Awesome @Zymantas! Where did you get/extract the language codes to add to your files? Anything else worth writing down for future updates of these files? |
I've taken language codes from the Xcode project localisations (where you add/remove translated languages of your app) and put them here. As I hoped, it worked on both platforms - iOS and macOS! :) I didn't find anything that's missing with these new languages, but I'll be retesting again when real-world translations will be ready (sometime next week as our translators promised). I'll let you know if this won't be merged till then. |
Not on macOS right now, could you maybe post a screenshot of the screen where you got the language codes from? Would appreciate it. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small change request! I think 😇
@janpio This can also be achieve by running |
Nice @joshdholtz - we should add that as a comment in the code where it can be used to "refresh" the list. |
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
Do we maybe also want to add comments that "interlink" these 3 files/locations @joshdholtz? Because if you update one of them, you should always update the others, correct? Can we also add some information in a comment where the other data comes from? |
Aye aye! Can do can do 💪 |
@janpio Updated with more info and link to this PR as an example 😊 |
Ok, I fixed the comment a bit and added a similar one to the second file. What about the JSON - we can't just add a comment there :/ What do we do there? But ohoh, I found another one: https://github.com/fastlane/fastlane/blob/master/spaceship/lib/assets/languageMappingReadable.json |
@janpio I did some searching through there and it doens't look like https://github.com/fastlane/fastlane/blob/master/spaceship/lib/assets/languageMappingReadable.json actually gets used anywhere. There are two methods that reference it ( Also going to ping @KrauseFx since he add this file back in 2015 😉 - fastlane-old/spaceship@dd51003#diff-355ef28abf38992915b1a2e1724703eb |
Deleteable code is best code ❌ 💀 |
Let's ship this 💪 |
Hey @Zymantas 👋 Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉 Please let us know if this change requires an immediate release by adding a comment here 👍 |
@@ -201,6 +243,12 @@ | |||
"es" | |||
] | |||
}, | |||
{ | |||
"locale": "sk-SK", | |||
"name": "Romanian", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshdholtz This should be Slovak
Ohoh. Can you please create another PR? Thanks. |
Ohhh :(. Since this is my PR, I'll create new one with a fix. |
Congratulations! 🎉 This was released as part of fastlane 2.116.1 🚀 |
Catalan, Croatian, Czech, Hindi, Hungarian, Polish, Romanian, Slovak, and Ukrainian
Checklist
bundle exec rspec
from the root directory to see all new and existing tests passbundle exec rubocop -a
to ensure the code style is validMotivation and Context
New App Store Connect languages were added some time ago and fastlane doesn't support them yet.
Description
Added new language constants and language mappings
close #13938