DIALIN-1
Dialin Review
Details
| Participant | Role | Time Spent | Comments | Latest Comment |
|---|---|---|---|---|
|
lbaloa
(deleted user)
|
Author & Moderator | 2h 26m | 17 | Replace with PAYLOAD_LENGTH_FIRST_PACKET setup to 3. Plea... |
| Reviewer - Complete | 1h 44m | 18 | Used constant. | |
| Reviewer - Complete | 1h 51m | 23 | RESOLVED | |
|
pmontazemi
(deleted user)
|
Reviewer - Complete | 1h 42m | 70 | RESOLVED. |
| Reviewer - Complete | 32m | 1 | RESOLVED | |
| Total | 8h 17m | 129 |
Branches in review
General Comments
Behrouz NematiPour
There are so many hex values roaming around the code which has not been defin...There are so many hex values roaming around the code which has not been defined in one place to be able to refer to.
Please collect them in one place with a line of description.
Or even if it is referring to the "X:\Engineering\Denali\06- Software Design\Messaging\Message List.xlsx" (which I found some there) please make a note to refer to that one if group of them can be read in that excel sheet.
-
Behrouz NematiPour
marked as
Resolved
10 Dec 19
Behrouz NematiPour
Is it our python coding standard to have an empty line at the begin of each i...Is it our python coding standard to have an empty line at the begin of each if block ?
I think that make code longer and hard to follow....
-
Behrouz NematiPour
marked as
Resolved
10 Dec 19
lbaloa (deleted user)
1) If you are referring to keyword arguments, it is allowed per our code stan...1) If you are referring to keyword arguments, it is allowed per our code standards and it is a standard in python.
2) Documentation for the Messaging list is being done in the excel file that you are referring. You will find that channel IDs are located DenaliChannels class. We need to do some touch ups in the excel document
3) Having a line after each if blocks is part of pep8.
Behrouz NematiPour
I don't like if that's pep8 (python coding style) or what ever We need to hav...I don't like if that's pep8 (python coding style) or what ever We need to have a subset of it for ourselves. and it's OK.
pmontazemi (deleted user)
Behrouz, If you are suggesting a change wrt PEP8 coding standard, that is fin...Behrouz,
If you are suggesting a change wrt PEP8 coding standard, that is fine. But, you will also need to make that change to our Python coding standard, which is currently an exact copy of the PEP8 coding standard for Python. What is the decision?
Behrouz NematiPour
Doesn't let me to click complete seem like I need to put a reply here. So : T...Doesn't let me to click complete seem like I need to put a reply here.
So :
The interpretation of PEP8 were not correct and Leo suppose to fix it.
RESOLVED
In general I'm not a fan of having parameter name when passing a value to it.
I know some languages like old lady VisualBasic had it, but it's confusing and also give the ability to reorder the functions parameters when passing values which I believe is not good and is making the code hard to understand and read.
As you used a very good naming for the values it shows which parameter is used.
Behrouz NematiPour marked as Resolved 10 Dec 19