|
Тема
|
ползата от code review
|
|
Автор |
~@!$^%*amp;()_+ (целия горен ред) |
Публикувано | 21.09.16 07:45 |
|
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/Android.mk
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/MessagesProcessor.h
File modules/vehicle/MessagesProcessor.h:
Line 9:
trailing spaces should be removed.
Line 18:
trailing spaces should be removed.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/VehicleHAL.cpp
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/VehicleHAL.h
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/VhalId.cpp
File modules/vehicle/VhalId.cpp:
Line 9:
remove trailing spaces.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/VhalId.h
File modules/vehicle/VhalId.h:
Line 17:
remove trailing spaces.
Line 18:
remove trailing spaces.
Line 21:
remove trailing spaces.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/ff_std_types.h
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/ff_utilities/ff_utility_endiannes.cpp
File modules/vehicle/ff_utilities/ff_utility_endiannes.cpp:
Line 7:
remove trailing spaces.
Line 10:
remove trailing spaces.
Line 11:
remove trailing spaces.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/ff_utilities/ff_utility_endiannes.h
File modules/vehicle/ff_utilities/ff_utility_endiannes.h:
Line 10:
remove trailing spaces.
Line 14:
remove trailing spaces.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/ff_utilities/ff_utility_file.cpp
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/ff_utilities/ff_utility_file.h
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/ff_utilities/ff_utility_logging.cpp
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/ff_utilities/ff_utility_logging.h
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
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/ff_utilities/ff_utility_networking.cpp
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/ff_utilities/ff_utility_networking.h
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/ff_utilities/ff_utility_string.cpp
File modules/vehicle/ff_utilities/ff_utility_string.cpp:
Line 6:
remove trailing spaces.
Line 25:
remove trailing spaces.
Line 53:
remove trailing spaces.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/ff_utilities/ff_utility_string.h
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/ff_utilities/ff_utility_thread.h
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/ff_utilities/ff_utility_timer.cpp
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/ff_utilities/ff_utility_udp_socket.cpp
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/ff_utilities/ff_utility_udp_socket.h
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/vhal.cpp
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.
http://minerva.devops.letv.com/#/c/42751/1/modules/vehicle/vhal_test/vhal_test.cpp
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
| |
|
кодревютата са като кръвопускането през средновековието - всички са убедени че има полза.
NE SUTOR ULTRA CREPIDAM
| |
|
аз редовно трия празните редове и слагам коментари защо. зле форматиран код говори много и за самото му качество.
Ако е писано - може и да се задомя, но не на всяка цена.
| |
|
Да не работиш в Минерва?
И не правильно думать, что есть чьим-то богом обещанный рай...
| |
|
Така е. Бил съм във фирма, където не само код ревю, но и още една камара "дисциплинарни" дейности се прилагаха стръвно. Сякаш можеха да подобрят програмата и избегнат грандиозните бъгове! В крайна сметка винаги се оказваше, че даден проблем е възникнал чисто и просто от безнадеждната некадърност на съответния програмист и никаква казармена дисциплина не би помогнала, докато по-кадърният персонал пцова под този бюрократичен терор.
~~~
Холистична кучешка храна.
(канадска измишльотина)
| |
|
| |
|
|
|
|