This is a list of all comments for UI-DEN-4438-1. Review Summary: No summary General Comment by plucia on 25 August 2020, 11:21 https://devapps.diality.us/cru/UI-DEN-4438-1#c3883 Non-production code has been added to the manager home and settings screens. Should this reside on the production master branch? Also, why is non-production code being unit tested? Reply by plucia on 27 August 2020, 12:33 > RESOLVED General Comment by Behrouz NematiPour on 25 August 2020, 12:42 https://devapps.diality.us/cru/UI-DEN-4438-1#c3889 The representation of the data is not the way it's going to be on production but the code that is being code reviewed and doing code coverage on it, is. All those codes will be logged, in the production code. ---------------------------------------- File: sources/gui/guiglobals.h Revision Comment by pmontazemi on 25 August 2020, 10:06 https://devapps.diality.us/cru/UI-DEN-4438-1#c3846 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:04 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:52 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:52 > RESOLVED. Revision Comment by pmontazemi on 25 August 2020, 09:57 https://devapps.diality.us/cru/UI-DEN-4438-1#c3825 Remove extra lines. Reply by Behrouz NematiPour on 25 August 2020, 13:38 > done Reply by pmontazemi on 28 August 2020, 09:42 > RESOLVED. ---------------------------------------- File: sources/view/hd/adjustment/VTreatmentAdjustmentSaline.h Revision Comment by pmontazemi on 20 August 2020, 21:57 https://devapps.diality.us/cru/UI-DEN-4438-1#c3703 Why is indentation of } off? Reply by Behrouz NematiPour on 24 August 2020, 00:05 > This is the namespace brace and the namespaces braces are not > indenting the code. so are at the same column as class brace > in this case. Reply by pmontazemi on 25 August 2020, 10:02 > OK, remove extra line between the } and respond to all my > other indentation comments with the same comment for me to > resolve. Reply by Behrouz NematiPour on 25 August 2020, 13:40 > removed extra line. > And put the same reply for the other related comments. Reply by pmontazemi on 28 August 2020, 09:43 > RESOLVED. ---------------------------------------- File: sources/canbus/messageinterpreter.cpp Revision Comment by pmontazemi on 25 August 2020, 10:06 https://devapps.diality.us/cru/UI-DEN-4438-1#c3842 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:02 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:54 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:54 > RESOLVED. ---------------------------------------- File: sources/gui/guiglobals.cpp Revision Comment by pmontazemi on 25 August 2020, 10:06 https://devapps.diality.us/cru/UI-DEN-4438-1#c3845 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:04 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:52 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:52 > RESOLVED. ---------------------------------------- File: sources/view/hd/data/VHDOperationModeData.h Revision Comment by pmontazemi on 20 August 2020, 21:57 https://devapps.diality.us/cru/UI-DEN-4438-1#c3704 Why is indentation of } off? Reply by Behrouz NematiPour on 24 August 2020, 00:05 > This is the namespace brace and the namespaces braces are not > indenting the code. so are at the same column as class brace > in this case. Reply by pmontazemi on 28 August 2020, 09:44 > RESOLVED. ---------------------------------------- File: sources/view/hd/data/VHDTreatmentStatesData.h Revision Comment by pmontazemi on 20 August 2020, 21:57 https://devapps.diality.us/cru/UI-DEN-4438-1#c3705 Why is indentation of } off? Reply by Behrouz NematiPour on 24 August 2020, 00:05 > This is the namespace brace and the namespaces braces are not > indenting the code. so are at the same column as class brace > in this case. Reply by pmontazemi on 28 August 2020, 09:44 > RESOLVED. ---------------------------------------- File: sources/view/hd/data/VTreatmentSalineData.h Revision Comment by pmontazemi on 20 August 2020, 21:58 https://devapps.diality.us/cru/UI-DEN-4438-1#c3706 1. Why is indentation of } off? 2. Also no extra line before } Reply by Behrouz NematiPour on 24 August 2020, 00:05 > 1 - This is the namespace brace and the namespaces braces are > not indenting the code. so are at the same column as class > brace in this case. > 2 - done Reply by pmontazemi on 25 August 2020, 10:03 > RESOLVED. ---------------------------------------- File: unittests/tst_messaging.cpp Revision Comment by pmontazemi on 20 August 2020, 21:59 https://devapps.diality.us/cru/UI-DEN-4438-1#c3709 Remove extra line. Reply by Behrouz NematiPour on 23 August 2020, 23:59 > done Reply by pmontazemi on 28 August 2020, 09:45 > RESOLVED. ---------------------------------------- File: unittests/tst_messaging.h Revision Comment by pmontazemi on 20 August 2020, 21:59 https://devapps.diality.us/cru/UI-DEN-4438-1#c3710 Remove extra line before } Reply by Behrouz NematiPour on 23 August 2020, 23:59 > done Reply by pmontazemi on 28 August 2020, 09:45 > RESOLVED. Revision Comment by pmontazemi on 20 August 2020, 21:59 https://devapps.diality.us/cru/UI-DEN-4438-1#c3711 Remove extra line at EOF. Reply by Behrouz NematiPour on 23 August 2020, 23:59 > done Reply by pmontazemi on 28 August 2020, 09:45 > RESOLVED. ---------------------------------------- File: sources/model/MAbstract.cpp Revision Comment by pmontazemi on 20 August 2020, 21:56 https://devapps.diality.us/cru/UI-DEN-4438-1#c3702 Replace FIXME with TODO, we only use TODO. Reply by Behrouz NematiPour on 25 August 2020, 13:46 > done Reply by pmontazemi on 28 August 2020, 09:43 > RESOLVED. ---------------------------------------- File: sources/view/vtreatmentadjustmentresponsebase.h Revision Comment by pmontazemi on 25 August 2020, 10:04 https://devapps.diality.us/cru/UI-DEN-4438-1#c3835 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:43 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:57 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:57 > RESOLVED. Revision Comment by pmontazemi on 20 August 2020, 21:58 https://devapps.diality.us/cru/UI-DEN-4438-1#c3707 Why is indentation of } off? Reply by Behrouz NematiPour on 24 August 2020, 00:04 > This is the namespace brace and the namespaces braces are not > indenting the code. so are at the same column as class brace > in this case. Reply by pmontazemi on 28 August 2020, 09:44 > RESOLVED. ---------------------------------------- File: sources/canbus/messagedispatcher.cpp Revision Comment by pmontazemi on 25 August 2020, 10:05 https://devapps.diality.us/cru/UI-DEN-4438-1#c3840 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:02 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:56 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:56 > RESOLVED. ---------------------------------------- File: sources/canbus/messageglobals.h Revision Comment by pmontazemi on 25 August 2020, 10:06 https://devapps.diality.us/cru/UI-DEN-4438-1#c3841 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:02 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:54 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:54 > RESOLVED. ---------------------------------------- File: sources/model/hd/adjustment/MTreatmentAdjustSalineResponse.h Revision Comment by pmontazemi on 20 August 2020, 21:56 https://devapps.diality.us/cru/UI-DEN-4438-1#c3701 Empty? Reply by Behrouz NematiPour on 24 August 2020, 00:06 > This is the vertical code alignment to make sure the > overloading is correctly done. > data() is not override from base class but is const and > fromByteArray() has to be overrode and should not be const. Reply by pmontazemi on 28 August 2020, 09:42 > RESOLVED. ---------------------------------------- File: sources/view/vtreatmentadjustmentultrafiltrationconfirm.h Revision Comment by pmontazemi on 25 August 2020, 10:04 https://devapps.diality.us/cru/UI-DEN-4438-1#c3832 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:43 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:58 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:58 > RESOLVED. Revision Comment by pmontazemi on 20 August 2020, 21:58 https://devapps.diality.us/cru/UI-DEN-4438-1#c3708 1. Why is indentation of } off? 2. Also no extra line between } Reply by Behrouz NematiPour on 24 August 2020, 00:01 > extra line removed. > This is the namespace brace and the namespaces braces are not > indenting the code. so are at the same column as class brace > in this case. Reply by pmontazemi on 25 August 2020, 10:04 > RESOLVED. ---------------------------------------- File: shared/scripts/names.py Revision Comment by pmontazemi on 20 August 2020, 22:01 https://devapps.diality.us/cru/UI-DEN-4438-1#c3712 Priority? (instead of Priory) Reply by Behrouz NematiPour on 23 August 2020, 23:58 > done Reply by pmontazemi on 25 August 2020, 10:09 > RESOLVED. ---------------------------------------- File: sources/storage/filehandler.cpp Revision Comment by pmontazemi on 25 August 2020, 10:05 https://devapps.diality.us/cru/UI-DEN-4438-1#c3837 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:05 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:57 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:57 > RESOLVED. ---------------------------------------- File: sources/view/VEventSpy.h Revision Comment by pmontazemi on 25 August 2020, 10:03 https://devapps.diality.us/cru/UI-DEN-4438-1#c3831 Remove extra line. Reply by Behrouz NematiPour on 25 August 2020, 13:52 > done Reply by pmontazemi on 28 August 2020, 09:41 > RESOLVED. ---------------------------------------- File: sources/canbus/caninterface.cpp Revision Comment by pmontazemi on 25 August 2020, 10:05 https://devapps.diality.us/cru/UI-DEN-4438-1#c3838 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 12:59 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:56 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:56 > RESOLVED. Revision Comment by pmontazemi on 25 August 2020, 09:55 https://devapps.diality.us/cru/UI-DEN-4438-1#c3822 Comment says 10K frames but we divide by 100000. Which one is correct? Also k is always spelled with lower case for kilo. Reply by Behrouz NematiPour on 25 August 2020, 13:36 > Thanks for your attention into the details. > I tweaked it many times to get that value and I should have > missed it. > done. Reply by pmontazemi on 28 August 2020, 09:57 > RESOLVED. ---------------------------------------- File: sources/canbus/frameinterface.cpp Revision Comment by pmontazemi on 25 August 2020, 10:05 https://devapps.diality.us/cru/UI-DEN-4438-1#c3839 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:01 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:56 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:56 > RESOLVED. ---------------------------------------- File: sources/applicationcontroller.cpp Revision Comment by pmontazemi on 25 August 2020, 10:07 https://devapps.diality.us/cru/UI-DEN-4438-1#c3849 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:43 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:51 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:51 > RESOLVED. ---------------------------------------- File: sources/applicationcontroller.h Revision Comment by pmontazemi on 25 August 2020, 10:07 https://devapps.diality.us/cru/UI-DEN-4438-1#c3850 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:44 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:51 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:51 > RESOLVED. Revision Comment by pmontazemi on 25 August 2020, 10:07 https://devapps.diality.us/cru/UI-DEN-4438-1#c3851 Remove extra line. Reply by Behrouz NematiPour on 25 August 2020, 13:44 > done Reply by pmontazemi on 28 August 2020, 09:41 > RESOLVED. ---------------------------------------- File: sources/gui/guicontroller.cpp Revision Comment by pmontazemi on 25 August 2020, 10:06 https://devapps.diality.us/cru/UI-DEN-4438-1#c3843 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:02 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:54 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:54 > RESOLVED. ---------------------------------------- File: sources/gui/guicontroller.h Revision Comment by pmontazemi on 25 August 2020, 10:06 https://devapps.diality.us/cru/UI-DEN-4438-1#c3844 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:02 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:53 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:53 > RESOLVED. Revision Comment by pmontazemi on 25 August 2020, 09:56 https://devapps.diality.us/cru/UI-DEN-4438-1#c3824 Remove extra line. Reply by Behrouz NematiPour on 25 August 2020, 13:38 > done Reply by pmontazemi on 28 August 2020, 09:56 > RESOLVED. Revision Comment by pmontazemi on 25 August 2020, 09:56 https://devapps.diality.us/cru/UI-DEN-4438-1#c3823 Fix indentation of } Reply by Behrouz NematiPour on 25 August 2020, 13:03 > We had discussed the namespace indentation in another comment > and resolved. > "This is the namespace brace and the namespaces braces are > not indenting the code. so are at the same column as class > brace in this case." Reply by pmontazemi on 28 August 2020, 09:57 > RESOLVED. ---------------------------------------- File: sources/gui/guiview.cpp Revision Comment by pmontazemi on 25 August 2020, 10:06 https://devapps.diality.us/cru/UI-DEN-4438-1#c3847 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:04 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:52 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:52 > RESOLVED. ---------------------------------------- File: sources/gui/guiview.h Revision Comment by pmontazemi on 25 August 2020, 10:06 https://devapps.diality.us/cru/UI-DEN-4438-1#c3848 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:05 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:51 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:51 > RESOLVED. Revision Comment by pmontazemi on 25 August 2020, 09:57 https://devapps.diality.us/cru/UI-DEN-4438-1#c3826 Remove extra line and fix indentation of } Reply by Behrouz NematiPour on 25 August 2020, 13:08 > This is the namespace brace and the namespaces braces are not > indenting the code. so are at the same column as class brace > in this case. Reply by Behrouz NematiPour on 25 August 2020, 13:38 > removed extra line. Reply by pmontazemi on 28 August 2020, 09:42 > RESOLVED. ---------------------------------------- File: sources/storage/DriveWatcher.h Revision Comment by pmontazemi on 25 August 2020, 09:58 https://devapps.diality.us/cru/UI-DEN-4438-1#c3827 Remove extra line and fix indentation of } Reply by Behrouz NematiPour on 25 August 2020, 13:10 > This is the namespace brace and the namespaces braces are not > indenting the code. so are at the same column as class brace > in this case. Reply by Behrouz NematiPour on 25 August 2020, 13:39 > removed extra line. Reply by pmontazemi on 28 August 2020, 09:41 > RESOLVED. ---------------------------------------- File: sources/storage/filehandler.h Revision Comment by pmontazemi on 25 August 2020, 10:05 https://devapps.diality.us/cru/UI-DEN-4438-1#c3836 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:06 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:57 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:57 > RESOLVED. Revision Comment by pmontazemi on 25 August 2020, 09:58 https://devapps.diality.us/cru/UI-DEN-4438-1#c3828 Remove extra line and fix indentation of }. Reply by Behrouz NematiPour on 25 August 2020, 13:11 > "This is the namespace brace and the namespaces braces are > not indenting the code. so are at the same column as class > brace in this case." Reply by pmontazemi on 28 August 2020, 09:57 > RESOLVED. ---------------------------------------- File: sources/view/valarmstatus.cpp Revision Comment by pmontazemi on 25 August 2020, 10:04 https://devapps.diality.us/cru/UI-DEN-4438-1#c3834 File name must use camelCase. Reply by Behrouz NematiPour on 25 August 2020, 13:43 > Absolutely, > I'm doing it incrementally and slowly to not to make the code > reviews harder and confusing git. > I did for so many and this file will get its turn soon. Reply by pmontazemi on 28 August 2020, 10:57 > OK, will be done part of Doxygenation Story. Reply by pmontazemi on 28 August 2020, 10:57 > RESOLVED. --- ID: UI-DEN-4438-1 https://devapps.diality.us/cru/UI-DEN-4438-1 Title: UI-DEN-4438_Saline Bolus Statement of Objectives: State: Closed Summary: Author: Behrouz NematiPour Reviewers: (0 active, 2 completed*) plucia (*) pmontazemi (*)