ползата от code review
5 отговора
0 прегледа
ползата от 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
(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
Re: ползата от code review
кодревютата са като кръвопускането през средновековието - всички са убедени че има полза.
NE SUTOR ULTRA CREPIDAM
NE SUTOR ULTRA CREPIDAM
Re: ползата от code review
аз редовно трия празните редове и слагам коментари защо. зле форматиран код говори много и за самото му качество.
Ако е писано - може и да се задомя, но не на всяка цена.
Ако е писано - може и да се задомя, но не на всяка цена.
Re: ползата от code review
Да не работиш в Минерва? 
И не правильно думать, что есть чьим-то богом обещанный рай...
И не правильно думать, что есть чьим-то богом обещанный рай...
Така е.
Така е. Бил съм във фирма, където не само код ревю, но и още една камара "дисциплинарни" дейности се прилагаха стръвно. Сякаш можеха да подобрят програмата и избегнат грандиозните бъгове! В крайна сметка винаги се оказваше, че даден проблем е възникнал чисто и просто от безнадеждната некадърност на съответния програмист и никаква казармена дисциплина не би помогнала, докато по-кадърният персонал пцова под този бюрократичен терор. 
~~~
Холистична кучешка храна.
(канадска измишльотина)
~~~
Холистична кучешка храна.
(канадска измишльотина)