Здравствуйте, Буравчик, Вы писали:
Б>Какие комментарии в код-ревью вы считаете "по делу", а какие нет? Б>Какими должны быть правильные комментарии к PR/MR/т.п.?
Б>(Навеяно темой в "коллеги улыбнитесь")
Здравствуйте, Буравчик, Вы писали:
Б>Какие комментарии в код-ревью вы считаете "по делу", а какие нет? Б>Какими должны быть правильные комментарии к PR/MR/т.п.?
По делу это те, которые (1) содержат информацию, что надо изменить (2) объясняют, почему это надо изменить и (3) говорят, как это изменить.
Здравствуйте, Pzz, Вы писали:
Б>>Какие комментарии в код-ревью вы считаете "по делу", а какие нет? Б>>Какими должны быть правильные комментарии к PR/MR/т.п.? Pzz>По делу это те, которые (1) содержат информацию, что надо изменить (2) объясняют, почему это надо изменить и (3) говорят, как это изменить.
(1) и (2) уже достаточно, с (3) это прям совсем хорошо.
Pzz>По делу это те, которые (1) содержат информацию, что надо изменить (2) объясняют, почему это надо изменить и (3) говорят, как это изменить.
Это идеальный коммент, настолько, что даже может быть code suggestion. Но это же обычно тривиальный коммент, практически линтер. В реальном же мире зачастую должно хватать коммента типа ай-ай-ай. Например, "где в этом коммите автотесты", "эта функциональность уже реализована в ХХХ", "мы же договорились — без костылей".
Здравствуйте, Sharov, Вы писали:
Pzz>>По делу это те, которые (1) содержат информацию, что надо изменить (2) объясняют, почему это надо изменить и (3) говорят, как это изменить.
S>(1) и (2) уже достаточно, с (3) это прям совсем хорошо.
(3) отфильтровывает комментарии из серии, что сделать было бы хорошо, но никто не знает, как.
Здравствуйте, SkyDance, Вы писали:
Pzz>>По делу это те, которые (1) содержат информацию, что надо изменить (2) объясняют, почему это надо изменить и (3) говорят, как это изменить.
SD>Это идеальный коммент, настолько, что даже может быть code suggestion. Но это же обычно тривиальный коммент, практически линтер. В реальном же мире зачастую должно хватать коммента типа ай-ай-ай. Например, "где в этом коммите автотесты", "эта функциональность уже реализована в ХХХ", "мы же договорились — без костылей".
В реальном мире получаешь 10 комментов про то, что хвостик у запятой не той формы, и редко-редко получаешь коммент по делу.
Здравствуйте, Буравчик, Вы писали:
Б>Какие комментарии в код-ревью вы считаете "по делу", а какие нет? Б>Какими должны быть правильные комментарии к PR/MR/т.п.?
Б>(Навеяно темой в "коллеги улыбнитесь")
Смотря какой code review. У нас все смотрит один человек и я если честно стараюсь поменьше с ним спорить, что бы быстрее его пройти. Ну только если в случае явных его косяков делаю возражения.
Pzz>(3) отфильтровывает комментарии из серии, что сделать было бы хорошо, но никто не знает, как.
Потому что это задача автора кода. Ревьювер знает специфику области ревью, и если автор кода ее не знает, то самое время узнать. Можно, конечно, заявить, что ревьюер еще должен быть и учителем. Но мы же ше в школе и не в детском саду. Взрослые люди должны уметь сами разобраться. Разумеется, если только между ревьюером и автором нет установленных отношений типа ментор-падаван.
Что до комментов "запятая не там", это не комменты, это линтеры, их замечания должны быть исправлены автоматически. Код-форматтеры и прочие инструменты уже давно придуманы.
Здравствуйте, Буравчик, Вы писали:
Б>Какие комментарии в код-ревью вы считаете "по делу", а какие нет? Б>Какими должны быть правильные комментарии к PR/MR/т.п.?
1. Баги. Это самое главное, конечно, тут обсуждать нечего.
2. Очевидно неверная архитектура кода. К примеру в используемых фреймворках есть "правильный" подход к решению этой проблемы, а автор по незнанию наворотил велосипедов.
3. Несоответствие принятому стилю кодирования. Это пункт спорный, я считаю, что подобные проблемы должны проверяться автоматическими инструментами, а также форматирование должно применяться автоматическими инструментами, т.е. при настроенном редакторе этой проблемы не должно возникать вообще. Но если с этой автоматикой не получилось подружиться, то так. Повторюсь, что пункт спорный и мелкие несоответствия я бы не отмечал, т.к. пользы от этого не так много, если в целом код нормальный.
Что, я считаю, не должно комментироваться никак: Несоответствие написанного кода идеалам ревьюера. К примеру в Java можно обработку коллекции сделать через for в императивном виде, а можно через streams в функциональном виде. Вот тут в 99% случаев получается чистая вкусовщина, оба варианта читабельные, оба варианта достаточно производительные, причём часто люди предпочитают один из них в своём коде. И тут может разгореться сражение тупоконечников с остроконечниками. Вот такую ситуацию допускать нельзя. Либо заранее принять стандарт кодирования через streams, к примеру, а лучше всего просто не заострять внимание на этом вопросе. Написано правильно, багов нет, и ладно.
В общем самое главное это чтобы код делал полезную работу без багов.
Менее главное это чтобы код соотвествовал нефункциональным, но принятым и задокументированным требованиям в данном проекте.
И совсем не важно, чтобы код соответствовал нефункциональным и не задокументированным требованиям.
Б>Какие комментарии в код-ревью вы считаете "по делу", а какие нет?
Цель "дела" — довольный расплатившийся заказчик. Соответственно, что ведёт к получению денег от заказчика, а что к удовлетворению ЧСВ одного ресурса за счёт безсмысленной работы другого...
