•  

Comment Results

Review Name Created Custom Fields Content
TESTSUITES-DEN-3724-1 15 Oct 2020

RESOLVED

TESTSUITES-DEN-3724-1 15 Oct 2020

RESOLVED

UI-DEN-4964-1 15 Oct 2020

RESOLVED.

UI-DEN-3605-4 30 Sep 2020

Why didn't use the attached scrollbar instead of non-attached?
regarding the Qt doc :
When using a non-attached ScrollBar, the following must be done manually:
When using a non-attached ScrollBar, the following must be done manually:
Layout the scroll bar (with the x and y or anchor properties, for example).
Set the size and position properties to determine the size and position of the scroll bar in relation to the scrolled item.
Set the active property to determine when the scroll bar will be visible.


your code could be as simple as :

Flickable {
...
 ScrollBar.vertical: ScrollBar { id: scrollBar }
...
}
HD-DEN-4308-3 30 Sep 2020

What are 80 and 85 comments? Imagine it is my first day on the job, how would I know what these are?

UI-DEN-3605-4 28 Aug 2020
UI-DEN-3605-4 30 Sep 2020

I believe you noticed that all the images should start with 'i' and with no extension.

UI-DEN-3605-4 28 Aug 2020

Done

HD-DEN-4211-1 28 Aug 2020

Moved to MessagePayloads.h for now to reduce code in an already large module. These payload definitions will eventually be distributed to the various modules that will be sending/handling the associated messages.

HD-DEN-4211-1 28 Aug 2020

Is this position value expected to change?

DIALIN-DEN-5053-1 25 Sep 2020

These should be placed in the common folder (common/msg_defs.py).

DIALIN-DEN-5053-1 25 Sep 2020

Should this be dialin to hd channel id?

UI-DEN-3605-4 09 Sep 2020

This model is for a request, so we're not parsing data from CAN, only sending it out.
There's no implementation of GetValue that works with QVariants. In other places in the code it's only used with QByteArrays.

DIALIN-DEN-4211-1 09 Sep 2020

For the new code, when you do the merge make sure that the message ID's are added to common/msg_defs.py instead of distributed to each class.
I have an existing ticket http://dvm-linux02:8080/browse/DIAL-33 to read the message IDs and alarm IDs from common for older code.

DIALIN-DEN-4690-1 07 Oct 2020

Added

UI-DEN-4964-1 13 Oct 2020

The m prefix is not informative / needed

DIALIN-DEN-5053-1 25 Sep 2020

Should this be dialin to hd channel id?

DIALIN-DEN-4690-1 07 Oct 2020

"*** almost off the subject ***"
it makes the parameter to be identified as a value, not a type.
there is no preference for function argument naming in python coding style.

DIALIN-DEN-4690-1 03 Oct 2020

"*** Off the subject ***"
Thanks for reminding that.
This is exactly what I've told and mentioned while ago at the beginning of adding these methods as part of Dialin, that was the main reason we did it.
As we talked while ago and you're the Dialin owner, please create a story for yourself and cooperate with our manager to define the priority/plan for the story.
From that moment on, all the newly added methods shall follow that design and implementation.

UI-DEN-4690-1 09 Sep 2020

This file has been replaced with its Camel Case version.
But since it had a copyright change on server which had no code review assigned to it Crucible is showing it here after I merged master into my branch.
So actually this file doesn't exist anymore.

HD-DEN-4308-3 16 Sep 2020

Done

HD-DEN-4308-3 22 Sep 2020

I know this is temporary test code, but global variables aren't allowed.

DG-DEN-3504-1 02 Nov 2020

RESOLVED in CODE WALKTHROUGH.

UI-DEN-4690-1 09 Sep 2020

This filename is not CamelCase

UI-DEN-4690-1 07 Oct 2020

"*** Off the subject **"

There are tests (manually/automatically) for that and the application quits properly.
If terminates by SquishCoco or being killed in the terminal may not have time to clean up correctly.
When shuts down by user within the UI by user quits nicely and clean.

"
** Also please don't check "Needs resolution" for every comment you make, it happened many times regardless of mentioning it multiple times ***"

UI-DEN-4690-1 30 Sep 2020

RESOLVED.

UI-DEN-4690-1 09 Sep 2020

This filename is not CamelCase

UI-DEN-4690-1 09 Sep 2020

This file is not in CamelCase

DG-DEN-3504-1 02 Nov 2020

RESOLVED in CODE WALKTHROUGH.

UI-DEN-3605-4 30 Sep 2020

Because it helped me debug my code during development

UI-DEN-4964-1 14 Oct 2020

No need for a duplicate conversation about this. RESOLVED.

UI-DEN-4964-1 02 Nov 2020

Addressed.

UI-DEN-4690-2-1 30 Sep 2020

The functions are missing docstrings.

Function descriptions and parameter types and descriptions, as well as the return type are needed

DIALIN-DEN-4308-1 30 Sep 2020

RESOLVED.

DIALIN-DEN-4308-1 28 Sep 2020

Remove extra line.

DG-DEN-3421-1 12 Oct 2020

should there be a P in front of I_CONTROLLER?

DIALIN-DEN-4308-1 30 Sep 2020

RESOLVED.

DIALIN-DEN-4308-1 30 Sep 2020

RESOLVED.

DIALIN-DEN-4308-1 28 Sep 2020

Remove extra line.

DIALIN-DEN-4308-1 28 Sep 2020

Align with left "(".

UI-DEN-4598-1 09 Nov 2020

RESOLVED

UI-DEN-4598-1 09 Nov 2020

RESOLVED

UI-DEN-4598-1 09 Nov 2020

RESOLVED

HD-DEN-4641-1 01 Dec 2020

RESOLVED in CODE WALKTHROUGH.

DIALIN-DEN-4308-1 30 Sep 2020

Any reason to keep this?

UI-DEN-3605-4 28 Aug 2020

Remove extra line.

UI-DEN-3605-4 28 Aug 2020

RESOLVED.

UI-DEN-3605-4 28 Aug 2020

Remove extra line.

UI-DEN-3605-4 28 Aug 2020

Done

DIALIN-DEN-5282-1 30 Oct 2020

Will use it in the next Story.
I need to ask you how to use it and I would require testing the code again.
Thanks for the help and suggestion it has been removed from here and used common in the testsuits.