This is a list of all comments for UI-DEN-2793-1.

Review Summary: No summary


----------------------------------------
File: sources/canbus/messageinterpreter.cpp
Revision Comment by pmontazemi on 26 April 2020, 14:20 
https://devapps.diality.us/cru/UI-DEN-2793-1#c1684

  Looking at our coding standard, cases within a switch are
  always indented. Looked also at latest and greatest coding
  standards online. Please indent all cases across the entire
  code base. This will make them not only consistent with our C++
  coding standard, but also more legible and compliant with the
  latest and greatest trends on C++ programming community.

  Reply by Behrouz NematiPour on 26 April 2020, 14:41
  > We are following Qt coding standard for most of our codes.
  > https://wiki.qt.io/Qt_Coding_Style
  > If our coding style documentation needs to be updated let me
  > know.

    Reply by pmontazemi on 26 April 2020, 14:50
    > From a quality and regulatory, what we see in the C++ code
    > must match Diality's C++ Coding Standard. What is why we
    > have that standard in place. Please create a Task for
    > yourself in DEN S18 with highest priority (No. 1) to update
    > the C++ coding standard. Please include Peter's and my time
    > for reviewing the changes, including release of the
    > document through Propel. Thanks!

      Reply by pmontazemi on 26 April 2020, 14:52
      > RESOLVED.

----------------------------------------
File: sources/gui/guiglobals.h
Revision Comment by pmontazemi on 26 April 2020, 14:22 
https://devapps.diality.us/cru/UI-DEN-2793-1#c1685

  Where these all these alarm enums got migrated to?

  Reply by Behrouz NematiPour on 26 April 2020, 14:33
  > Common repo.
  > This is the purpose of the using common repo/

    Reply by pmontazemi on 26 April 2020, 14:48
    > RESOLVED.

----------------------------------------
File: unittests/tst_utilities.cpp
Revision Comment by pmontazemi on 26 April 2020, 14:26 
https://devapps.diality.us/cru/UI-DEN-2793-1#c1687

  Why are we adding "_" in some function names and in some other
  cases with just stick the names one after the other without
  "_"?

  Reply by Behrouz NematiPour on 26 April 2020, 14:35
  > sorry, I don't understand the question !?

    Reply by pmontazemi on 26 April 2020, 21:08
    > Is this annotation part of the Qt C++ coding standard?
    > tst_fromvariant_Bool_True? What is the rule? I am more used
    > to:
    > 
    > tstFromVariantBoolTrue, which is camelCase.

      Reply by pmontazemi on 26 April 2020, 21:11
      > Understood, you have replied this question in another
      > part of this code review.

        Reply by pmontazemi on 26 April 2020, 21:11
        > RESOLVED

Revision Comment by pmontazemi on 26 April 2020, 14:26 
https://devapps.diality.us/cru/UI-DEN-2793-1#c1688

  Here is a mix of "_", all lower caps, and camelCase?!?

  Reply by Behrouz NematiPour on 26 April 2020, 14:36
  > the test names follow the pattern below :
  > tst_<function name>_<test case> if is a unit test for a
  > specific function.
  > and function names start with non capital cases.

    Reply by pmontazemi on 26 April 2020, 21:09
    > RESOLVED

----------------------------------------
File: unittests/tst_views.cpp
Revision Comment by pmontazemi on 26 April 2020, 14:27 
https://devapps.diality.us/cru/UI-DEN-2793-1#c1689

  Here is ALL CAPS with "_" inbetween. What part of the C++
  coding standard states this?

  Reply by Behrouz NematiPour on 26 April 2020, 14:31
  > This is not my code and it has been defined in common.
  > If I change it FW will break

    Reply by pmontazemi on 26 April 2020, 21:10
    > Understood, this is all CAPITAL because it is commonly used
    > with FW, which is consistent with our C Coding Standard.

      Reply by pmontazemi on 26 April 2020, 21:10
      > RESOLVED

----------------------------------------
File: sources/view/vtreatmentadjustmentsresponse.cpp
Revision Comment by pmontazemi on 26 April 2020, 14:23 
https://devapps.diality.us/cru/UI-DEN-2793-1#c1686

  With respect to our C++ coding standard (section 8.13), we must
  use camelCase for all function names.

  Reply by Behrouz NematiPour on 26 April 2020, 14:33
  > it's the file name.
  > Sure for the function names.

    Reply by pmontazemi on 26 April 2020, 21:07
    > RESOLVED


---

ID: UI-DEN-2793-1 https://devapps.diality.us/cru/UI-DEN-2793-1

Title: UI-DEN-2793_AddCommonRepositoryToTheUIProjectAndRefactorCurrentCodes
Statement of Objectives:


State: Closed

Summary: 

Author: Dara Navaei
Moderator: Behrouz NematiPour
Reviewers: (0 active, 2 completed*)
     plucia   (*)
     pmontazemi   (*)