Skip to content

Conversation

@samuelnguyen999
Copy link
Contributor

No description provided.

Copy link
Member

@prestoncarman prestoncarman left a comment

Choose a reason for hiding this comment

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

There are a few unnecessary changes in this PR. Remove those changes.

Also, it would be nice to have tests for the menu changes. Since we are making big menu changes, I think other tests should be failing and we would need to clean those up.

The file for the SetupCalibration class
"""
from titration.devices.library import Keypad
from titration.ui_state.calibration.calibrate_ph import CalibratePh
Copy link
Member

Choose a reason for hiding this comment

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

I think you should either leave this file unchanged or put in your stub.

@@ -0,0 +1,57 @@
"""
Copy link
Member

Choose a reason for hiding this comment

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

Why is this file different from the stub version? I think you should put in your stub. We can later copy over code from Titration.

@@ -1,82 +1,237 @@
"""
Copy link
Member

Choose a reason for hiding this comment

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

We did not discuss tests, but we should have a set of tests for this menu.

The previous tests are located here: https://github.com/Open-Acidification/TankControllerPython/blob/main/test/ui_state/main_menu_test.py

Please update the tests and make sure we have working set of tests.

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.

2 participants