-
Notifications
You must be signed in to change notification settings - Fork 45
chore: add crowdin entries automagically [WPB-8645] #4518
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
base: develop
Are you sure you want to change the base?
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #4518 +/- ##
===========================================
- Coverage 48.98% 48.69% -0.29%
===========================================
Files 565 571 +6
Lines 19561 19677 +116
Branches 3258 3276 +18
===========================================
Hits 9582 9582
- Misses 8980 9096 +116
Partials 999 999 see 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
yamilmedina
left a comment
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.
Looks good to me; codewise, it's clear and nice usage of conventions plugin 🙌
My only concern is, is it perhaps a solution for a problem we don't have? since there is no guaranteed or 100% failproof and configuring a module is more or less tasks we don't do everyday.
|
It's guaranteed to catch at some point. When the app is built, for example. There is currently no automation to detect misconfiguration / missing entries in Being pragmatic, I think the alternatives are:
I think this one doesn't hurt and can be easily customisable or disabled if needed in the future, without breaking anything. I don't like the idea of keeping project-specific extra steps if they can be avoided. Adding Gradle modules should be easy and hassle free.
It could be more aggressive as well, and do it at configuration phase. Not sure if better or worse. |



PR Submission Checklist for internal contributors
The PR Title
SQPIT-764The PR Description
What's new in this PR?
Issues
Some Crowdin entries are missing from configuration.
Causes
We need to remember to add it manually when we create new modules.
Solutions
Add new entries automatically when building each module.
_moduleName_/src/main/res/values/string.xmlexists./crowdin.yml.The task is added to all
package*Resourcestasks. So if the module is built at any point, it will trigger the task.Note
This is NOT 100% failproof, but it's super good enough:
Someone could create a new module, and still raise a PR and merge it without building the app, or running the
addEntryToCrowdinFileGradle task.However, anyone else building the app later would notice changes to crowdin configuration, and they could be committed in the future without issues.
Testing
Ran locally, missing entries are added by the new task, not by me.
How to Test
crowdin.yml../gradlew addEntryToCrowdinFile; OR build the app / build one library module.crowdin.yml.PR Post Merge Checklist for internal contributors
References
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.