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

Controller cleanup: lazy imports no longer required #496

Merged
merged 2 commits into from
Feb 12, 2024

Conversation

kdmukai
Copy link
Contributor

@kdmukai kdmukai commented Sep 23, 2023

Description

At an earlier phase of the test suite refactor (a few months back) and before all the mocking had been put in to wall off the hardware-dependent libraries, the Controller was updated to use lazy imports (not explicitly importing via standard from blah import foo).

But with the test suite in its current form, this workaround is no longer necessary.

This pull request is categorized as a:

  • Code refactor

Checklist

  • I’ve run pytest and made sure all unit tests pass before sumbitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

  • N/A

I have tested this PR on the following platforms/os:

Note: Keep your changes limited in scope; if you uncover other issues or improvements along the way, ideally submit those as a separate PR. The more complicated the PR the harder to review, test, and merge.

@jdlcdl
Copy link

jdlcdl commented Sep 23, 2023

ACK tested

  • no problems passing test suite and screenshot generator on linux desktop and raspi-os on pi2
  • no problems doing a few random operations in seedsigner for raspi-os on pi2 and ss-os on pi0 w/ reproducible image 14a4ad3de2a661cf37631651f12cc3dcd96925297a338743e9b9816421f3a58b

@newtonick
Copy link
Collaborator

ACK, LGTM. Tested without any issues.

@newtonick newtonick merged commit a73cae6 into SeedSigner:dev Feb 12, 2024
1 check passed
@kdmukai kdmukai deleted the controller_cleanup branch September 2, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants