This is a list of all comments for DIALIN-DEN-2379-1.

Review Summary: No summary

General Comment by plucia on 01 July 2020, 16:29 
https://devapps.diality.us/cru/DIALIN-DEN-2379-1#c2656

  I think other commits of mine and others are still included in
  this review that aren't related. Can you point me to the files
  that you changed?

  Reply by Dara Navaei on 01 July 2020, 16:36
  > Ok, please review heaters.py and temperature_sensors.py


----------------------------------------
File: dialin/dg/heaters.py
Revision Comment by plucia on 01 July 2020, 16:47 
https://devapps.diality.us/cru/DIALIN-DEN-2379-1#c2667

  Just need to subclass _AbstractSubSystem, use epytext
  formatting in all functions, make handler_heaters_sync private
  (e.g. _handler_heaters_sync), and decorate
  _handler_headers_sync with the _publish decorator, with a list
  of all class variables that will be published to observers.

  Reply by Dara Navaei on 07 July 2020, 09:00
  > Done

    Reply by plucia on 07 July 2020, 16:35
    > RESOLVED

Revision Comment by plucia on 01 July 2020, 16:50 
https://devapps.diality.us/cru/DIALIN-DEN-2379-1#c2668

  Our python coding standard requires lower_case_with_underscores
  for variable names and function names

  Reply by Dara Navaei on 07 July 2020, 09:10
  > Done

    Reply by plucia on 07 July 2020, 16:34
    > RESOLVED

----------------------------------------
File: dialin/dg/temperature_sensors.py
Revision Comment by plucia on 01 July 2020, 16:37 
https://devapps.diality.us/cru/DIALIN-DEN-2379-1#c2659

  Should inhereit from _AbstractSubSystem

  Reply by Dara Navaei on 07 July 2020, 08:49
  > Done

    Reply by plucia on 07 July 2020, 16:38
    > RESOLVED

Revision Comment by plucia on 01 July 2020, 16:38 
https://devapps.diality.us/cru/DIALIN-DEN-2379-1#c2660

  Add super().__init__() after inheriting from _AbstractSubSystem

  Reply by Dara Navaei on 07 July 2020, 08:50
  > Done

    Reply by plucia on 07 July 2020, 16:38
    > RESOLVED

Revision Comment by plucia on 01 July 2020, 16:51 
https://devapps.diality.us/cru/DIALIN-DEN-2379-1#c2669

  Our python coding standard requires lower_case_with_underscores
  for variable names and function names.

  Reply by Dara Navaei on 07 July 2020, 09:10
  > Done

    Reply by plucia on 07 July 2020, 16:36
    > RESOLVED

Revision Comment by plucia on 01 July 2020, 16:38 
https://devapps.diality.us/cru/DIALIN-DEN-2379-1#c2661

  handler_temperature_sensors should be private
  e.g.: _handler_temperature_sensors()

  Reply by plucia on 01 July 2020, 16:43
  > Make sure to decorate your _handler functions with the
  > _publish decorator with a list of strings for all class
  > variables that will be published.

    Reply by Dara Navaei on 07 July 2020, 08:57
    > Done

      Reply by plucia on 07 July 2020, 16:37
      > RESOLVED

Revision Comment by plucia on 01 July 2020, 16:39 
https://devapps.diality.us/cru/DIALIN-DEN-2379-1#c2662

  Use epytext docstring format instead
  
  We're no longer using doxygen in dialin

  Reply by Dara Navaei on 07 July 2020, 08:52
  > Done

    Reply by plucia on 07 July 2020, 16:37
    > RESOLVED

Revision Comment by plucia on 01 July 2020, 16:41 
https://devapps.diality.us/cru/DIALIN-DEN-2379-1#c2664

  Needs to be in epytext format so sphinx will pick it up

  Reply by Dara Navaei on 07 July 2020, 08:51
  > Done

    Reply by plucia on 07 July 2020, 16:38
    > RESOLVED

Revision Comment by plucia on 01 July 2020, 16:41 
https://devapps.diality.us/cru/DIALIN-DEN-2379-1#c2663

  Needs to be in epytext format so sphinx will pick it up

  Reply by Dara Navaei on 07 July 2020, 08:52
  > Done

    Reply by plucia on 07 July 2020, 16:38
    > RESOLVED

----------------------------------------
File: dialin/dg/dialysate_generator.py
Revision Comment by Sean Nash on 07 July 2020, 08:21 
https://devapps.diality.us/cru/DIALIN-DEN-2379-1#c2703

  Why are these commented out?

  Reply by Dara Navaei on 07 July 2020, 09:11
  > They had been commented for testing. I uncommented them
  > again.

    Reply by Sean Nash on 09 July 2020, 07:57
    > They are still commented out.

      Reply by Sean Nash on 16 July 2020, 17:02
      > Dara resolved in another branch.  RESOLVED.

----------------------------------------
File: dialin/hd/rtc.py
Revision Comment by Sean Nash on 07 July 2020, 08:26 
https://devapps.diality.us/cru/DIALIN-DEN-2379-1#c2704

  Will DG have an RTC class as well?

  Reply by Dara Navaei on 07 July 2020, 09:12
  > Eventually yes.  I have a TODO note to add RTC to DG as well.

    Reply by Sean Nash on 09 July 2020, 07:55
    > RESOLVED.


---

ID: DIALIN-DEN-2379-1 https://devapps.diality.us/cru/DIALIN-DEN-2379-1

Title: DIALIN-DEN-2379_DG Dialysate Temperature (1 of 2)
Statement of Objectives:


State: Closed

Summary: 

Author: Dara Navaei
Moderator: Dara Navaei
Reviewers: (1 active, 2 completed*)
     Sean Nash   (*)
     plucia   (*)
     pmontazemi