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.

Sign up for GitHub

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

Jump to bottom

updating flashlight version #1100

Merged
merged 3 commits into from
Jun 17, 2024
Merged

updating flashlight version #1100

merged 3 commits into from
Jun 17, 2024

Conversation

WendelHime
Copy link
Contributor

  • New code should not be using FutureBuilder (use ValueListenableBuilder instead)
  • 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”?

@WendelHime WendelHime marked this pull request as ready for review June 14, 2024 20:32
@WendelHime WendelHime requested review from hwh33 and garmr-ulfr June 14, 2024 20:34
hwh33
hwh33 approved these changes Jun 17, 2024
View reviewed changes
Copy link
Contributor

@hwh33 hwh33 left a comment

Choose a reason for hiding this comment

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

LGTM!

garmr-ulfr
garmr-ulfr approved these changes Jun 17, 2024
View reviewed changes
Copy link
Contributor

@garmr-ulfr garmr-ulfr left a comment

Choose a reason for hiding this comment

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

Other than reverting the go version, this looks good to me. You can go ahead an merge after updating the version.

go.mod Outdated Show resolved Hide resolved
@WendelHime WendelHime merged commit d170683 into main Jun 17, 2024
1 of 2 checks passed
@WendelHime WendelHime deleted the feat/1380 branch June 17, 2024 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garmr-ulfr garmr-ulfr garmr-ulfr approved these changes

@hwh33 hwh33 hwh33 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.
3 participants
@WendelHime @hwh33 @garmr-ulfr