openedx / openedx-app-ios

The mobile app for iOS for the Open EdX Platform.
Apache License 2.0
19 stars 13 forks source link

chore: clean up in-code uk translations | FC-55 #465

Open OmarIthawi opened 2 weeks ago

OmarIthawi commented 2 weeks ago

Clean up after #422

make pull_translations should be used

openedx-webhooks commented 2 weeks ago

Thanks for the pull request, @OmarIthawi! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

This is currently a draft pull request. When it is ready for our review and all tests are green, click "Ready for Review", or remove "WIP" from the title, as appropriate.

volodymyr-chekyrta commented 1 week ago

@OmarIthawi, could you please fix the tests? You've deleted files, but something is still looking for them.

The following build commands failed:
    CopyStringsFile /Users/runner/Library/Developer/Xcode/DerivedData/OpenEdX-bsdxmseilanfjjcjomwdntdeffpi/Build/Products/DebugDev-iphonesimulator/Core.framework/uk.lproj/Localizable.strings /Users/runner/work/openedx-app-ios/openedx-app-ios/Core/Core/uk.lproj/Localizable.strings (in target 'Core' from project 'Core')
OmarIthawi commented 1 week ago

@OmarIthawi, could you please fix the tests? You've deleted files, but something is still looking for them.

The following build commands failed:
  CopyStringsFile /Users/runner/Library/Developer/Xcode/DerivedData/OpenEdX-bsdxmseilanfjjcjomwdntdeffpi/Build/Products/DebugDev-iphonesimulator/Core.framework/uk.lproj/Localizable.strings /Users/runner/work/openedx-app-ios/openedx-app-ios/Core/Core/uk.lproj/Localizable.strings (in target 'Core' from project 'Core')

I tried to fix the failing test but I'm unsure how to find the step CopyStringsFile. Could you please help?

@volodymyr-chekyrta Please try to checkout the branch, run Project -> Clean and it should fix the issue. I don't have XCode on my Linux machine which makes this not possible.

volodymyr-chekyrta commented 1 week ago

@OmarIthawi, could you please fix the tests? You've deleted files, but something is still looking for them.

The following build commands failed:
    CopyStringsFile /Users/runner/Library/Developer/Xcode/DerivedData/OpenEdX-bsdxmseilanfjjcjomwdntdeffpi/Build/Products/DebugDev-iphonesimulator/Core.framework/uk.lproj/Localizable.strings /Users/runner/work/openedx-app-ios/openedx-app-ios/Core/Core/uk.lproj/Localizable.strings (in target 'Core' from project 'Core')

I tried to fix the failing test but I'm unsure how to find the step CopyStringsFile. Could you please help?

@volodymyr-chekyrta Please try to checkout the branch, run Project -> Clean and it should fix the issue. I don't have XCode on my Linux machine which makes this not possible.

I tried this, but the project build fails. The files are not deleted correctly because the project files are still referenced; you have to remove them from XCode. Does anyone on your team have XCode? Or maybe I can help you with this.

image image
OmarIthawi commented 1 week ago

@volodymyr-chekyrta please help, that would be really appreciated.

We only need to delete those files now they're backed to Transifex

Our iOS engineer is juggling a couple of tasks already and I need her focused on SAML SSO :) and I'm filling up gaps whenever I can.

volodymyr-chekyrta commented 1 week ago

@volodymyr-chekyrta please help, that would be really appreciated.

We only need to delete those files now they're backed to Transifex

Our iOS engineer is juggling a couple of tasks already and I need her focused on SAML SSO :) and I'm filling up gaps whenever I can.

Could you please give me write permissions to this branch in your repo? I'll fix it and push changes

OmarIthawi commented 1 week ago

@volodymyr-chekyrta you now have write access to the branches. I think we need to revert the change and remove the files manually again via the XCode UI.

volodymyr-chekyrta commented 1 week ago

@OmarIthawi Okay, that approach won't work :( If we remove links from the translation files and then pull them via make pull_translations, files will be physically on the disk, but the project will not know about them.

image image
OmarIthawi commented 1 week ago

Okay, that approach won't work :(

@volodymyr-chekyrta That's an issue. How did we manage the tx pull in previous versions of the application such as:

I suppose people didn't just manually edit those files, did they?

OmarIthawi commented 1 week ago

@volodymyr-chekyrta I've checked about the issue more and I have some ideas:

Option 1: Use xcString files

I'm not an iOS expert, but if the xcstrings can fit all translations in a single file, I suppose that'll help.

Which is as far as I understand, uses one JSON file for all languages.

Option 2: Automate adding the files

The other alternative it to automatically add the uk and other languages after pulling them via Transifex. This can be done via libraries like https://github.com/kronenthaler/mod-pbxproj

OmarIthawi commented 1 week ago

@volodymyr-chekyrta I've made some progress regarding the XCode project file edits, I'll pickup the work again in a day or two.

volodymyr-chekyrta commented 3 days ago

@OmarIthawi, seems the script adding files to the wrong location

Screenshot 2024-06-25 at 9 40 41
volodymyr-chekyrta commented 3 days ago

It should be like this

image
OmarIthawi commented 3 days ago

That's a bumper, a pretty bad one. Thanks for checking @volodymyr-chekyrta.

I'll study the problem more and get back to you. I'm not ready to give up yet 😅