Здравствуйте, Буравчик, Вы писали:
Б>Какие комментарии в код-ревью вы считаете "по делу", а какие нет? Б>Какими должны быть правильные комментарии к 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, к примеру, а лучше всего просто не заострять внимание на этом вопросе. Написано правильно, багов нет, и ладно.
В общем самое главное это чтобы код делал полезную работу без багов.
Менее главное это чтобы код соотвествовал нефункциональным, но принятым и задокументированным требованиям в данном проекте.
И совсем не важно, чтобы код соответствовал нефункциональным и не задокументированным требованиям.
Б>Какие комментарии в код-ревью вы считаете "по делу", а какие нет?
Цель "дела" — довольный расплатившийся заказчик. Соответственно, что ведёт к получению денег от заказчика, а что к удовлетворению ЧСВ одного ресурса за счёт безсмысленной работы другого...
Друга ищи не того, кто любезен с тобой, кто с тобой соглашается, а крепкого советника, кто полезного для тебя ищет и противится твоим необдуманным словам.
Здравствуйте, Pzz, Вы писали:
Pzz>Здравствуйте, SkyDance, Вы писали:
Pzz>>>По делу это те, которые (1) содержат информацию, что надо изменить (2) объясняют, почему это надо изменить и (3) говорят, как это изменить.
SD>>Это идеальный коммент, настолько, что даже может быть code suggestion. Но это же обычно тривиальный коммент, практически линтер. В реальном же мире зачастую должно хватать коммента типа ай-ай-ай. Например, "где в этом коммите автотесты", "эта функциональность уже реализована в ХХХ", "мы же договорились — без костылей".
Pzz>В реальном мире получаешь 10 комментов про то, что хвостик у запятой не той формы, и редко-редко получаешь коммент по делу.
может уровень ревьюера ниже просто и по делу сказать нечего?
Здравствуйте, Буравчик, Вы писали:
Б>Какие комментарии в код-ревью вы считаете "по делу", а какие нет? Б>Какими должны быть правильные комментарии к PR/MR/т.п.?
Б>(Навеяно темой в "коллеги улыбнитесь")
Вообще считаю код-ревью плохой практикой. Комментарии надо собирать, когда код ещё пишется, а не когда код уже написан. Например, тимлид может несколько часов в неделю посидеть с каждым. Или люди могут периодически работать вместе, показывать друг другу свой код. А когда код написан, и его надо переписывать, это приводит к прохождению через все стадии гнева/отрицания/торга/принятия, а это отнимает силы и мотивацию. Жаль, что мало команд умеет обходиться без этого всего.
Здравствуйте, Alekzander, Вы писали:
A>Вообще считаю код-ревью плохой практикой. Комментарии надо собирать, когда код ещё пишется, а не когда код уже написан. Например, тимлид может несколько часов в неделю посидеть с каждым. Или люди могут периодически работать вместе, показывать друг другу свой код.
Так обычно и бывает:
— либо команда уже понимает принципы реализации тех и иных задач
— либо тимлид рассказывает свое видение реализации
— либо исполнитель (случае сомнений) согласовывает его с тимлидом и/или другими членами команды
Да и ревьювить надо даже, если предварительно обо всем договорились.
A>А когда код написан, и его надо переписывать, это приводит к прохождению через все стадии гнева/отрицания/торга/принятия, а это отнимает силы и мотивацию.
Код во время ревью не надо полностью переписывать, а надо улучшать.
Т.е. общий подход к решение задачи будет верный (см. выше), но нужно изменить некоторые детали. Поэтому редко нужен "гнев/отрицание/торг/принятие".
A>Жаль, что мало команд умеет обходиться без этого всего.
Здравствуйте, Буравчик, Вы писали:
Б>Так обычно и бывает: Б>- либо команда уже понимает принципы реализации тех и иных задач Б>- либо тимлид рассказывает свое видение реализации Б>- либо исполнитель (случае сомнений) согласовывает его с тимлидом и/или другими членами команды
Это всё не то. Надо не "вИдение" обсуждать, а периодически вместе код писать. Тогда его можно не ревьюить. Зачем ревьюить код, если его уже видело как минимум два разных человека?
Здравствуйте, Alekzander, Вы писали:
A>Это всё не то. Надо не "вИдение" обсуждать, а периодически вместе код писать. Тогда его можно не ревьюить. Зачем ревьюить код, если его уже видело как минимум два разных человека?
Здравствуйте, Буравчик, Вы писали:
A>>Это всё не то. Надо не "вИдение" обсуждать, а периодически вместе код писать. Тогда его можно не ревьюить. Зачем ревьюить код, если его уже видело как минимум два разных человека?
Б>Вдвоем код писать — дорого. В отличие от ревью
Зато и результат лучше, и в итоге себя оправдывает.
Но да, я не совсем удачно выразился. Надо не просто чтобы команда умела в подобные коммуникации, но и контора не была почасовой потогонкой.
Здравствуйте, Буравчик, Вы писали:
Б>Какие комментарии в код-ревью вы считаете "по делу", а какие нет? Б>Какими должны быть правильные комментарии к PR/MR/т.п.?
Б>(Навеяно темой в "коллеги улыбнитесь")
Кодревью нужен в первую очередь для того, чтобы не пропустить код, где
* явные ошибки — чаще всего по невнимательности или по причине температуры/недосыпа у автора
* непонятного/запутанного кода — в голове у автора какая-то картинка имеется, и он из-за этого под действием "проклятия знания" (остальным непонятно/сложно)
Ещё кодревью нужен, чтобы кто-то другой был тоже в курсе, что задача решалась/решена. Иногда на ревью вылезает, что некоторую подзадачу вообще не надо было решать — для неё уже существовало решение.
----------------
Вот от этого и пляшем. Всё остальное от лукавого, и даже более того — демотивирует и разваливает коллективы.
Всё сказанное выше — личное мнение, если не указано обратное.
