ползата от code review

5 отговора 0 прегледа
#1

ползата от code review

Patch Set 1:

(151 comments)

Some general comments:
1. trailing spaces should be removed. I tried mark all of them, but I may missed some.
2. Indent should use spaces instead of tab.
3. ff copyright header need be added to the source code.
These are coding format we need first correct.

[линк]
File modules/vehicle/Android.mk:

Line 13: .
This is FF property, not Android project, we should replace this header with FF version.


Line 22:
trailing spaces should be removed.


Line 23:
trailing spaces should be removed.


Line 33:
trailing spaces should be removed.


Line 34:
trailing spaces should be removed.


Line 35:
trailing spaces should be removed.


Line 39:
trailing spaces should be removed.


Line 64:
trailing spaces should be removed.


Line 73:
trailing spaces should be removed.


Line 74:
trailing spaces should be removed.


[линк]
File modules/vehicle/MessagesProcessor.h:

Line 9:
trailing spaces should be removed.


Line 18:
trailing spaces should be removed.


[линк]
File modules/vehicle/VehicleHAL.cpp:

Line 24:
remove trailing spaces.


Line 26:
remove trailing spaces.


Line 33:
remove trailing spaces.


Line 40:
remove trailing spaces.


Line 49:
remove trailing spaces.


Line 51:
remove trailing spaces.


Line 53:
remove trailing spaces.


Line 63:
remove trailing spaces.


Line 71:
remove trailing spaces.


Line 78:
remove trailing spaces.


Line 80:
remove trailing spaces.


Line 84:
remove trailing spaces.


[линк]
File modules/vehicle/VehicleHAL.h:

Line 23:
trailing spaces should be removed.


Line 25:
trailing spaces should be removed.


Line 33:
trailing spaces should be removed.


[линк]
File modules/vehicle/VhalId.cpp:

Line 9:
remove trailing spaces.


[линк]
File modules/vehicle/VhalId.h:

Line 17:
remove trailing spaces.


Line 18:
remove trailing spaces.


Line 21:
remove trailing spaces.


[линк]
File modules/vehicle/ff_std_types.h:

Line 9: */
should use standard ff copyright header instead of this one.
author's name will be kept in the revision history of git.


[линк]
File modules/vehicle/ff_utilities/ff_utility_endiannes.cpp:

Line 7:
remove trailing spaces.


Line 10:
remove trailing spaces.


Line 11:
remove trailing spaces.


[линк]
File modules/vehicle/ff_utilities/ff_utility_endiannes.h:

Line 10:
remove trailing spaces.


Line 14:
remove trailing spaces.


[линк]
File modules/vehicle/ff_utilities/ff_utility_file.cpp:

Line 13:
remove trailing spaces.


Line 16:
remove trailing spaces.


Line 17:
remove trailing spaces.


Line 24:
remove trailing spaces.


Line 56:
remove trailing spaces.


Line 57:
remove trailing spaces.


Line 117:
remove trailing spaces.


Line 130:
remove trailing spaces.


Line 137:
remove trailing spaces.


Line 140:
remove trailing spaces.


Line 161:
remove trailing spaces.


Line 162:
remove trailing spaces.


Line 167:
remove trailing spaces.


[линк]
File modules/vehicle/ff_utilities/ff_utility_file.h:

Line 19:
remove trailing spaces.


Line 42:
remove trailing spaces.


Line 49:
remove trailing spaces.


Line 50:
remove trailing spaces.


Line 53:
remove trailing spaces.


Line 54:
remove trailing spaces.


[линк]
File modules/vehicle/ff_utilities/ff_utility_logging.cpp:

Line 7:
remove trailing spaces.


Line 12:
remove trailing spaces.


Line 13:
remove trailing spaces.


Line 18:
remove trailing spaces.


[линк]
File modules/vehicle/ff_utilities/ff_utility_logging.h:

Line 16:
remove trailing spaces


Line 20:
remove trailing spaces


Line 21:
remove trailing spaces


Line 29:
remove trailing spaces


Line 34:
remove trailing spaces


Line 35:
remove trailing spaces


Line 37:
remove trailing spaces


[линк]
File modules/vehicle/ff_utilities/ff_utility_networking.cpp:

Line 8:
remove trailing spaces.


Line 10:
remove trailing spaces.


Line 16:
remove trailing spaces.


Line 32:
remove trailing spaces.


Line 38:
remove trailing spaces.


Line 39:
remove trailing spaces.


Line 40:
remove trailing spaces.


Line 44:
remove trailing spaces.


[линк]
File modules/vehicle/ff_utilities/ff_utility_networking.h:

Line 17:
remove trailing spaces.


Line 26:
remove trailing spaces.


Line 34:
remove trailing spaces.


Line 35:
remove trailing spaces.


[линк]
File modules/vehicle/ff_utilities/ff_utility_string.cpp:

Line 6:
remove trailing spaces.


Line 25:
remove trailing spaces.


Line 53:
remove trailing spaces.


[линк]
File modules/vehicle/ff_utilities/ff_utility_string.h:

Line 17:
remove trailing spaces.


Line 21:
remove trailing spaces.


Line 22:
remove trailing spaces.


Line 24:
remove trailing spaces.


Line 26:
remove trailing spaces.


Line 27:
remove trailing spaces.


[линк]
File modules/vehicle/ff_utilities/ff_utility_thread.h:

Line 17:
please use space instead of tab here.


Line 18:
please use space instead of tab here.


Line 24:
please use space instead of tab here.


Line 25:
please use space instead of tab here.


Line 33:
please use space instead of tab here.


Line 37:
please use space instead of tab here.


[линк]
File modules/vehicle/ff_utilities/ff_utility_timer.cpp:

Line 35:
please use space instead of tab here.


Line 36:
please use space instead of tab here.


Line 41:
please use space instead of tab here.


Line 51:
please use space instead of tab here.


[линк]
File modules/vehicle/ff_utilities/ff_utility_udp_socket.cpp:

Line 9:
remove trailing spaces.


Line 12:
remove trailing spaces.


Line 13:
remove trailing spaces.


Line 14:
remove trailing spaces.


Line 15:
remove trailing spaces.


Line 24:
remove trailing spaces.


Line 25:
remove trailing spaces.


Line 29:
remove trailing spaces.


Line 35:
remove trailing spaces.


Line 41:
remove trailing spaces.


Line 43:
Indent should be 4 white spaces.


Line 71:
remove trailing spaces.


Line 79:
remove trailing spaces.


Line 84:
remove trailing spaces.


[линк]
File modules/vehicle/ff_utilities/ff_utility_udp_socket.h:

Line 27:
remove trailing spaces.


Line 29:
remove trailing spaces.


Line 33:
remove trailing spaces.


Line 34:
remove trailing spaces.


Line 36:
remove trailing spaces.


Line 40:
remove trailing spaces.


Line 42:
remove trailing spaces.


Line 45:
remove trailing spaces.


[линк]
File modules/vehicle/vhal.cpp:

Line 14:
remove trailing spaces.


Line 53:
remove trailing spaces.


Line 65:
remove trailing spaces.


Line 84:
remove trailing spaces.


Line 86:
remove trailing spaces.


Line 88:
remove trailing spaces.


Line 89:
remove trailing spaces.


Line 90:
remove trailing spaces.


Line 91:
remove trailing spaces.


Line 93:
remove trailing spaces.


Line 94:
remove trailing spaces.


Line 96:
remove trailing spaces.


Line 97:
remove trailing spaces.


Line 98:
remove trailing spaces.


Line 100:
remove trailing spaces.


Line 101:
remove trailing spaces.


Line 105:
remove trailing spaces.


Line 108:
remove trailing spaces.


Line 111:
remove trailing spaces.


Line 115:
remove trailing spaces.


Line 122:
remove trailing spaces.


Line 123:
remove trailing spaces.


Line 125:
remove trailing spaces.


Line 127:
remove trailing spaces.


Line 138:
remove trailing spaces.


[линк]
File modules/vehicle/vhal_test/vhal_test.cpp:

Line 8:
use space instead of tab.


Line 9:
remove trailing spaces.


Line 13:
remove trailing spaces.


Line 14:
remove trailing spaces.


Line 19:
remove trailing spaces.


Line 20:
use space instead of tab

Отговор
в отговор на ~@!$^%*amp;()_+ #2

Re: ползата от code review

кодревютата са като кръвопускането през средновековието - всички са убедени че има полза.




NE SUTOR ULTRA CREPIDAM
Отговор
в отговор на ~@!$^%*amp;()_+ #3

Re: ползата от code review

аз редовно трия празните редове и слагам коментари защо. зле форматиран код говори много и за самото му качество.

Ако е писано - може и да се задомя, но не на всяка цена.
Отговор
в отговор на ~@!$^%*amp;()_+ #4

Re: ползата от code review

Да не работиш в Минерва?

И не правильно думать, что есть чьим-то богом обещанный рай...
Отговор
в отговор на zaphod #5

Така е.

Така е. Бил съм във фирма, където не само код ревю, но и още една камара "дисциплинарни" дейности се прилагаха стръвно. Сякаш можеха да подобрят програмата и избегнат грандиозните бъгове! В крайна сметка винаги се оказваше, че даден проблем е възникнал чисто и просто от безнадеждната некадърност на съответния програмист и никаква казармена дисциплина не би помогнала, докато по-кадърният персонал пцова под този бюрократичен терор.

~~~
Холистична кучешка храна.
(канадска измишльотина)
Отговор