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.

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

refactor(core): consistent naming of UI models #4479

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

obrusvit
Copy link
Contributor

@obrusvit obrusvit commented Jan 8, 2025

  • UI layouts name changes:
    • model_tt -> layout_bolt
    • model_tr -> layout_samson
    • model_mercury -> layout_quicksilver
  • rust features model_xyz freed for different use, now it's
    layout_xyz
  • input_flow function names are based on UI layout and not internal
    model name (i.e. quicksilver instead of t3t1)
  • directory names and commentary changed accordingly

[no changelog]

@obrusvit obrusvit added core Trezor Core firmware. Runs on Trezor Model T and T2B1. code Code improvements labels Jan 8, 2025
@obrusvit obrusvit self-assigned this Jan 8, 2025
@obrusvit obrusvit requested review from mmilata and removed request for prusnak January 8, 2025 16:28
Copy link

github-actions bot commented Jan 8, 2025

core UI changes device test click test persistence test
T2T1 Model T test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3B1 Safe 3 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3T1 Safe 5 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
All main(screens)

@obrusvit
Copy link
Contributor Author

obrusvit commented Jan 8, 2025

Needed for FIDO2 tests: trezor/fido2-tests#6

@TychoVrahe
Copy link
Contributor

instead of model_xx, can we use ui_xx? model means something different, and we will need model_xx features eventually too as a separate set.

@prusnak
Copy link
Member

prusnak commented Jan 8, 2025

instead of model_xx, can we use ui_xx?

I guess this makes more sense.

Also, maybe I am late to the party - but since you are renaming stuff, have you thought about different naming scheme? American car brands are OK, but maybe we could come up with something more Czech? Cities, fictional characters, etc.
But of course, if we need to do the change ASAP, let's not block the process.

@mmilata
Copy link
Member

mmilata commented Jan 8, 2025

fictional car brands https://mafiagame.fandom.com/wiki/Vehicles_in_Mafia#Vehicles

Copy link
Member

@mmilata mmilata left a comment

Choose a reason for hiding this comment

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

please also update the fido2-tests submodule

@obrusvit
Copy link
Contributor Author

obrusvit commented Jan 9, 2025

instead of model_xx, can we use ui_xx? model means something different, and we will need model_xx features eventually too as a separate set.

Yes, except we have lots of features ui_something, e.g. ui_debug, ui_antialiasing, etc.
How about ui_impl_xx or gui_xx?

Also, maybe I am late to the party - but since you are renaming stuff, have you thought about different naming scheme? American car brands are OK, but maybe we could come up with something more Czech? Cities, fictional characters, etc.
But of course, if we need to do the change ASAP, let's not block the process.

Hmm, I wouldn't mind something different but "mercury" and "lincoln" are a part of everyday vocabulary within the team at this point :D

@TychoVrahe
Copy link
Contributor

maybe layout_xx, which is consistent with python? but everything is fine if its not model :)

@obrusvit obrusvit force-pushed the obrusvit/ui-models-naming branch from f145d8a to dc10be4 Compare January 9, 2025 15:07
@obrusvit
Copy link
Contributor Author

obrusvit commented Jan 9, 2025

@prusnak After internal discussion with the team, we rename the individual UI layout implementation after the car brands from czech game series Mafia 😆

The names were chosen from this page: https://mafiagame.fandom.com/wiki/Vehicle_Brands
e.g. Ford is Bolt in Mafia

@obrusvit obrusvit added the translations Put this label on a PR to run tests in all languages label Jan 9, 2025
- UI layouts name changes:
  - model_tt -> layout_bolt
  - model_tr -> layout_samson
  - model_mercury -> layout_quicksilver
- rust features `model_xyz` freed for different use, now it's
`layout_xyz`
- input_flow function names are based on UI layout and not internal
model name (i.e. quicksilver instead of t3t1)
- directory names and commentary changed accordingly

[no changelog]
@obrusvit obrusvit force-pushed the obrusvit/ui-models-naming branch from dc10be4 to 7a1305f Compare January 9, 2025 16:51
@obrusvit obrusvit merged commit 862c987 into main Jan 9, 2025
139 of 140 checks passed
@obrusvit obrusvit deleted the obrusvit/ui-models-naming branch January 9, 2025 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code Code improvements core Trezor Core firmware. Runs on Trezor Model T and T2B1. translations Put this label on a PR to run tests in all languages
Projects
Status: 🤝 Needs QA
Development

Successfully merging this pull request may close these issues.

4 participants