height in inches and feet

[daveffr]

Right now, a patient’s height on a workup can’t be input in both feet and inches at the same time. I added a field to the workup model to make this possible, then made migrations.

[wwick]

Hi, David. I appreciate your work on this, and sorry that I'm just now reviewing.

I appreciate that this is a small change. At my work, our general rule is that the great majority of changes should be <100 lines, and changes >200 lines won't be reviewed. This allows early feedback and higher quality feedback since you can be more focused on each line that has changed. So good job on that part.

Some points of feedback:

1. It's not clear to my what problem this PR is trying to solve, and why this was selected as the approach. I assume you're looking for a way for clinics to easily enter height in feet and inches, but the PR summary should make this clear.
2. It's best practice from a systems design perspective to keep backend and frontend as separate as possible. In this case, how users input a patient's height/weight, and how that height is stored in the backend should not depend on one another.
3. This change does not include unit tests, and it doesn't include pictures or other end-to-end proof that it is doing what it is supposed to.
4. From what I can tell from the generated migration file, this would delete all previous height fields from the database, which doesn't seem right to me. Regardless, since the migration would act on a production database at some point, you should test with sample databases to make sure it's doing what you intend.