Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Build Info PR Suggestions #585

Merged
merged 2 commits into from
Nov 29, 2021
Merged

Build Info PR Suggestions #585

merged 2 commits into from
Nov 29, 2021

Conversation

oxtoacart
Copy link

  • All strings are defined using localized message keys like 'my_message_key'.i18n and are defined in en.po.
  • All colors are defined using Hex (not using built-in colors), are defined in colors.dart and are not duplicated
  • All text styles are defined in text_styles.dart and are not duplicated
  • All icons are using the vector resource from Figma (do not use built-in Icons)
  • Repeated code has been factored into custom widgets
  • Layout looks good in both LTR (English) and RTL (Persian) languages
  • If you refactored existing code, have you tested the refactored functionality against the old version to make sure you didn't break anything?
  • Do the tests pass? Consistently?
  • Did this change improve test coverage?
  • Is the code in question being linted? If not, consider adding a linter step to CI. If yes, make sure the linter is happy.
  • Have you logged tickets for related technical debt with the label “techdebt”?

oxtoacart
oxtoacart commented Nov 29, 2021
View reviewed changes
assets/locales/en.po
@@ -74,6 +74,12 @@ msgstr "Proxy everything"
msgid "proxy_everything_is"
msgstr "Proxy everything is %s"

msgid "version_number"
Copy link
Author

Choose a reason for hiding this comment

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

Localized these strings

oxtoacart
oxtoacart commented Nov 29, 2021
View reviewed changes
lib/app.dart
builder: (BuildContext context, AsyncSnapshot<void> snapshot) {
if (!snapshot.hasData) {
Copy link
Author

Choose a reason for hiding this comment

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

Drive-by, don't display UI until translations are loaded.

@oxtoacart oxtoacart merged commit 3188be1 into kr/buildInfo Nov 29, 2021
@oxtoacart oxtoacart deleted the ox/buildInfo branch November 29, 2021 15:43
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@cosmicespresso cosmicespresso cosmicespresso approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@oxtoacart @cosmicespresso