Как вежливо ревьювить код?
От: #John Европа https://github.com/ichensky
Дата: 20.04.21 10:33
Оценка:
Здравствуйте,
как вежливо делать замечания по коду, желательно с примерами на англ.

Как лучше просить что-то добавить?
"Would you, please, ..."
"Will you, please, ..."
"Could you, please, ..."
"How about to .. "
"Is this ... done?"

...

Как вежливо попросить что-то переписать, потому что код выглядит не очень?

"The code will be look much better if ..., what do you think?"
Re: Как вежливо ревьювить код?
От: _ABC_  
Дата: 20.04.21 10:36
Оценка:
Здравствуйте, #John, Вы писали:

J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?

J>"The code will be look much better if ..., what do you think?"
А официально принятые в рамках организации или проекта гайдлайны по коду есть?
Re: Как вежливо ревьювить код?
От: gyraboo Россия  
Дата: 20.04.21 10:41
Оценка:
Здравствуйте, #John, Вы писали:

J>Здравствуйте,

J>как вежливо делать замечания по коду, желательно с примерами на англ.

J>Как лучше просить что-то добавить?

J>"Would you, please, ..."
J>"Will you, please, ..."
J>"Could you, please, ..."
J>"How about to .. "
J>"Is this ... done?"

J>...


J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?


J>"The code will be look much better if ..., what do you think?"


You are awesome, but this is mthfk bullshit.

А если серьезно, ты не заколебешся на каждое замечание писать polite тексты? Не лучше ли на дейли всех хвалить за их работу, и говорить что код ревью — это совместная работа и т.д., что все молодцы, что замечания на код ревью не призваны никого оскорбить или наругать. А в замечаниях просто и лаконично писать по делу?
www.nanonewsnet.ru
Re: Как вежливо ревьювить код?
От: AleksandrN Россия  
Дата: 20.04.21 11:21
Оценка:
Здравствуйте, #John, Вы писали:

J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?


"выглядит не очень" — слишком размытая характеристика. Что под этим подразумевается?
Если есть объективные проблемы в коде — их и описать.
Re: Как вежливо ревьювить код?
От: fmiracle  
Дата: 20.04.21 11:33
Оценка:
Здравствуйте, #John, Вы писали:

J>как вежливо делать замечания по коду


1. делать вежливо. Ну в смысле не говорить, что это вот как-то по-мудацки написано, а говорить, ну, напирмер, что этот фрагмент сложный для понимания
2. не делать замечания не относящиеся к коду. Т.е. если бы ты написал этот код иначе, чем написал автор, но написанное автором решает задачу, укладывается в требования и гайдлайны компании, если они есть, и в целом решение не хуже чем то, которое предпочел бы ты — то тут и не место замечаниям.

J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?


Указать конкретные проблемы в чем "не очень", если они требуют переписывания.
Не забывай, что все эти переписывания — это время и силы. Их нужно тратить, если от этого улучшается код и не нужно иначе. Соответственно — следует обосновать, зачем человеку тратить на это силы.
Re[2]: Как вежливо ревьювить код?
От: gyraboo Россия  
Дата: 20.04.21 11:45
Оценка:
Здравствуйте, fmiracle, Вы писали:

J>>как вежливо делать замечания по коду


F>1. делать вежливо. Ну в смысле не говорить, что это вот как-то по-мудацки написано, а говорить, ну, напирмер, что этот фрагмент сложный для понимания

F>2. не делать замечания не относящиеся к коду. Т.е. если бы ты написал этот код иначе, чем написал автор, но написанное автором решает задачу, укладывается в требования и гайдлайны компании, если они есть, и в целом решение не хуже чем то, которое предпочел бы ты — то тут и не место замечаниям.

J>>Как вежливо попросить что-то переписать, потому что код выглядит не очень?


F>Указать конкретные проблемы в чем "не очень", если они требуют переписывания.

F>Не забывай, что все эти переписывания — это время и силы. Их нужно тратить, если от этого улучшается код и не нужно иначе. Соответственно — следует обосновать, зачем человеку тратить на это силы.

У команды должен быть чек-лист со сводом правил, и чтобы все разрабы были с ним согласны. Например, правило: применяем SOLID. Или: применяем clean code мартина и макконнела. Или: применяем GoF при необходимости. Испольщуем полиморфизм вместо switch в бизнес-логике. Работам по контрактоному варианту. Применяем DDD или же просто анемичную модель. И т.д. и т.п. Этот лист должен постоянно дополняться, чтобы отражать консенсус всей команды. В этом случае не придется постоянно дискутировать и аргументировать свою позицию, будет достаточно дать ссылку на нарушенное в чек-листе правило, а не писать мини-лекции в каждом замечании.
www.nanonewsnet.ru
Re: Как вежливо ревьювить код?
От: scf  
Дата: 20.04.21 11:57
Оценка: 10 (4) +4
Здравствуйте, #John, Вы писали:

J>Здравствуйте,

J>как вежливо делать замечания по коду, желательно с примерами на англ.

Есть хорошая статья на SO https://stackoverflow.blog/2019/09/30/how-to-make-good-code-reviews-better/

Вкратце — замечания лучше оформлять в виде вопросов. Не указали final — Shouldn't this be final? Переусложнили — Won't using xxx simplify this code? Забыли какой-то кейс — What about yyy?
Re[3]: Как вежливо ревьювить код?
От: fmiracle  
Дата: 20.04.21 12:02
Оценка:
Здравствуйте, gyraboo, Вы писали:

J>>>Как вежливо попросить что-то переписать, потому что код выглядит не очень?

F>>Указать конкретные проблемы в чем "не очень", если они требуют переписывания.
F>>Не забывай, что все эти переписывания — это время и силы. Их нужно тратить, если от этого улучшается код и не нужно иначе. Соответственно — следует обосновать, зачем человеку тратить на это силы.

G>У команды должен быть чек-лист со сводом правил, и чтобы все разрабы были с ним согласны. Например, правило: применяем SOLID. Или: применяем clean code мартина и макконнела. Или: применяем GoF при необходимости. Испольщуем полиморфизм вместо switch в бизнес-логике. Работам по контрактоному варианту. Применяем DDD или же просто анемичную модель. И т.д. и т.п. Этот лист должен постоянно дополняться, чтобы отражать консенсус всей команды. В этом случае не придется постоянно дискутировать и аргументировать свою позицию, будет достаточно дать ссылку на нарушенное в чек-листе правило, а не писать мини-лекции в каждом замечании.


0. Я полностью тут с тобой согласен
при этом
1. обращение к справочнику проблем и практик — это тоже вариант обоснования же
2. и оно все равно может порождать споры есть тут нарушение или нет и требовать прояснения ПОЧЕМУ тут на твой взгляд происходит нарушение. Ну, в смысле если принято писать в SOLID и автор знает, что так принято, и написал иначе — то значит или он вредитель или он считает что написал правильно и просто сказать "у тебя тут нарушение SOLID" без пояснений — мало что даст же
Re[4]: Как вежливо ревьювить код?
От: gyraboo Россия  
Дата: 20.04.21 12:11
Оценка:
Здравствуйте, fmiracle, Вы писали:

J>>>>Как вежливо попросить что-то переписать, потому что код выглядит не очень?

F>>>Указать конкретные проблемы в чем "не очень", если они требуют переписывания.
F>>>Не забывай, что все эти переписывания — это время и силы. Их нужно тратить, если от этого улучшается код и не нужно иначе. Соответственно — следует обосновать, зачем человеку тратить на это силы.

G>>У команды должен быть чек-лист со сводом правил, и чтобы все разрабы были с ним согласны. Например, правило: применяем SOLID. Или: применяем clean code мартина и макконнела. Или: применяем GoF при необходимости. Испольщуем полиморфизм вместо switch в бизнес-логике. Работам по контрактоному варианту. Применяем DDD или же просто анемичную модель. И т.д. и т.п. Этот лист должен постоянно дополняться, чтобы отражать консенсус всей команды. В этом случае не придется постоянно дискутировать и аргументировать свою позицию, будет достаточно дать ссылку на нарушенное в чек-листе правило, а не писать мини-лекции в каждом замечании.


F>0. Я полностью тут с тобой согласен

F>при этом
F>1. обращение к справочнику проблем и практик — это тоже вариант обоснования же
F>2. и оно все равно может порождать споры есть тут нарушение или нет и требовать прояснения ПОЧЕМУ тут на твой взгляд происходит нарушение. Ну, в смысле если принято писать в SOLID и автор знает, что так принято, и написал иначе — то значит или он вредитель или он считает что написал правильно и просто сказать "у тебя тут нарушение SOLID" без пояснений — мало что даст же

SOLID — достаточно объективный набор принципов. Их нарушения в 90% видны объективно. Проблемы с SOLID и споры обычно бывают из-за того, что кто-то не совсем корректно понимает некоторые принципы. Поэтому да, часто приходится не просто указывать на нарушение SOLID, но и читать мини-лекцию прямо в замечании. Это обычно для джунов и мидлов делается и является неплохим вариантом их обучения "на реальных примерах", и такие временные издержки должны конечно же закладываться в оценку сроков и эта практика должна являться явной и "официальной" частью культуры разработки, а не делаться изподтишка или абы как.
www.nanonewsnet.ru
Отредактировано 20.04.2021 12:13 gyraboo . Предыдущая версия . Еще …
Отредактировано 20.04.2021 12:12 gyraboo . Предыдущая версия .
Re: Как вежливо ревьювить код?
От: TimurSPB Интернет  
Дата: 20.04.21 12:33
Оценка:
J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?
Можно спросить "почему ты решил именно скопировать эту часть кода, а не выделить функцию? Создал таск чтобы разобраться." вместо "з??;л уже копипастить! деклайн!"
Make flame.politics Great Again!
Re: Как вежливо ревьювить код?
От: velkin Удмуртия http://blogs.rsdn.org/effective/
Дата: 20.04.21 13:49
Оценка:
Здравствуйте, #John, Вы писали:

J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?


Напомнило мне как в почтовые письма автоматически вставляется текст, вроде с уважением ля ля ля. Хотя лично я такое не люблю так как по сути это ненужная для прочтения информация.
Re: Как вежливо ревьювить код?
От: Zhendos  
Дата: 20.04.21 14:04
Оценка:
Здравствуйте, #John, Вы писали:

J>Здравствуйте,

J>как вежливо делать замечания по коду, желательно с примерами на англ.

J>Как лучше просить что-то добавить?

J>"Would you, please, ..."
J>"Will you, please, ..."
J>"Could you, please, ..."
J>"How about to .. "
J>"Is this ... done?"

J>...


J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?


J>"The code will be look much better if ..., what do you think?"


Можно посмотреть кучу примеров в больших проектах с открытым исходным кодом.
Например Qt: https://codereview.qt-project.org/c/qt/qtbase/+/343993

> I'd like to have some opinions on this design decision. Should we require the use of
Re[5]: Как вежливо ревьювить код?
От: IT Россия linq2db.com
Дата: 20.04.21 14:15
Оценка:
Здравствуйте, gyraboo, Вы писали:

G>SOLID — достаточно объективный набор принципов. Их нарушения в 90% видны объективно. Проблемы с SOLID и споры обычно бывают из-за того, что кто-то не совсем корректно понимает некоторые принципы. Поэтому да, часто приходится не просто указывать на нарушение SOLID, но и читать мини-лекцию прямо в замечании. Это обычно для джунов и мидлов делается и является неплохим вариантом их обучения "на реальных примерах", и такие временные издержки должны конечно же закладываться в оценку сроков и эта практика должна являться явной и "официальной" частью культуры разработки, а не делаться изподтишка или абы как.


Только не забудь сказать джунам, что если дело дошло до применения SOLID, то в коде уже проблемы, не смотря на то будут или нет применены принципы SOLID
//rsdn.org/forum/images/bis.gif Если нам не помогут, то мы тоже никого не пощадим.
Re: Как вежливо ревьювить код?
От: hi_octane Беларусь  
Дата: 20.04.21 14:17
Оценка: 2 (1) +1
J>Как лучше просить что-то добавить?
J>"Would you, please, ..."
J>"Will you, please, ..."
J>"Could you, please, ..."
J>"How about to .. "
J>"Is this ... done?"
Давно читал в англоязычном блоге, помню плохо, а сейчас не смог найти. Но вроде некоторые обороты, которые в обычной жизни могут вполне использоваться, в корпоративной лексике имеют другой смысл. Например "could you" в контексте ревью может читаться ими как агрессивное "способен ли ты" с уклоном в "сомневаюсь" а "Is this ... done?" вопрос уровня "наставник ученику", т.е. корректен если ты какой лид, или сеньёр ревьювящий за джуном. А для равных между собой он используется только когда вы уже враги которые друг друга валят при первой возможности. Злорадное "вот ты и попался! вот тут нот дан!".
Короче это надо спрашивать не тут, а на форуме нейтивных нейтивов, живущих офисной жизнью.
Re[6]: Как вежливо ревьювить код?
От: gyraboo Россия  
Дата: 20.04.21 14:17
Оценка:
Здравствуйте, IT, Вы писали:

G>>SOLID — достаточно объективный набор принципов. Их нарушения в 90% видны объективно. Проблемы с SOLID и споры обычно бывают из-за того, что кто-то не совсем корректно понимает некоторые принципы. Поэтому да, часто приходится не просто указывать на нарушение SOLID, но и читать мини-лекцию прямо в замечании. Это обычно для джунов и мидлов делается и является неплохим вариантом их обучения "на реальных примерах", и такие временные издержки должны конечно же закладываться в оценку сроков и эта практика должна являться явной и "официальной" частью культуры разработки, а не делаться изподтишка или абы как.


IT>Только не забудь сказать джунам, что если дело дошло до применения SOLID, то в коде уже проблемы, не смотря на то будут или нет применены принципы SOLID


Почему? Не улавливаю мысль. Конкретный пример можешь привести, где после применения SOLID остаются проблемы?
www.nanonewsnet.ru
Re[7]: Как вежливо ревьювить код?
От: IT Россия linq2db.com
Дата: 20.04.21 14:28
Оценка:
Здравствуйте, gyraboo, Вы писали:

IT>>Только не забудь сказать джунам, что если дело дошло до применения SOLID, то в коде уже проблемы, не смотря на то будут или нет применены принципы SOLID

G>Почему? Не улавливаю мысль. Конкретный пример можешь привести, где после применения SOLID остаются проблемы?

Не после применения, а до.
Возьмём тот же LSP, который гласит:

объекты в программе должны быть заменяемыми на экземпляры их подтипов без изменения правильности выполнения программы


Когда возможно нарушение этого принципа? Например, в случае если у нас имеется иерархия объектов с развесистым и запутанным интерфейсом наследования. Принцип предписывает быть внимательнее с такими иерархиями и наследованиями. Но если дело вообще не доводить до таких иерархий, то не потребуется и сам принцип.
//rsdn.org/forum/images/bis.gif Если нам не помогут, то мы тоже никого не пощадим.
Re[8]: Как вежливо ревьювить код?
От: gyraboo Россия  
Дата: 20.04.21 14:31
Оценка:
Здравствуйте, IT, Вы писали:

IT>>>Только не забудь сказать джунам, что если дело дошло до применения SOLID, то в коде уже проблемы, не смотря на то будут или нет применены принципы SOLID

G>>Почему? Не улавливаю мысль. Конкретный пример можешь привести, где после применения SOLID остаются проблемы?

IT>Не после применения, а до.

IT>Возьмём тот же LSP, который гласит:

IT>

объекты в программе должны быть заменяемыми на экземпляры их подтипов без изменения правильности выполнения программы


IT>Когда возможно нарушение этого принципа? Например, в случае если у нас имеется иерархия объектов с развесистым и запутанным интерфейсом наследования. Принцип предписывает быть внимательнее с такими иерархиями и наследованиями. Но если дело вообще не доводить до таких иерархий, то не потребуется и сам принцип.


Ну не совсем соглашусь. LSP нарушается, и у меня на памяти свежи такие нарушения, даже там, где есть очень простая иерархия: один абстрактный класс или интерфейс и всего 2-3 реализации.

А насчет сложных иерархий — согласен. Ну на это уже есть и этой бочке затычка другие принципы: заменяйте наследование композицией, используйте магическое число 7 (а лучше меньше, как ограничение кратковременной памяти), KISS.
www.nanonewsnet.ru
Отредактировано 20.04.2021 14:33 gyraboo . Предыдущая версия .
Re[2]: Как вежливо ревьювить код?
От: Lonely Dog Россия  
Дата: 20.04.21 14:38
Оценка:
Здравствуйте, scf, Вы писали:

scf>Здравствуйте, #John, Вы писали:


J>>Здравствуйте,

J>>как вежливо делать замечания по коду, желательно с примерами на англ.

scf>Есть хорошая статья на SO https://stackoverflow.blog/2019/09/30/how-to-make-good-code-reviews-better/


scf>Вкратце — замечания лучше оформлять в виде вопросов. Не указали final — Shouldn't this be final? Переусложнили — Won't using xxx simplify this code? Забыли какой-то кейс — What about yyy?

Примерно так и стараюсь делать. Иногда пишу что-то типа 'looks like breaking change (details here) please explain' или 'seems to be performance issue (details here) please explain'. Крайне редко пишу жестко (если это не первая попытка ревью, а человек не понимает).
Re[9]: Как вежливо ревьювить код?
От: IT Россия linq2db.com
Дата: 20.04.21 14:44
Оценка:
Здравствуйте, gyraboo, Вы писали:

G>Ну не совсем соглашусь. LSP нарушается, и у меня на памяти свежи такие нарушения, даже там, где есть очень простая иерархия: один абстрактный класс или интерфейс и всего 2-3 реализации.


Это как раз и есть подтверждение моих слов. Если тебе или кому-то на твоей памяти удалось такое спроектировать, то проблема вовсе не в нарушении LSP. Наверняка там ещё то порождение "творца".
//rsdn.org/forum/images/bis.gif Если нам не помогут, то мы тоже никого не пощадим.
Re[10]: Как вежливо ревьювить код?
От: gyraboo Россия  
Дата: 20.04.21 14:49
Оценка: :)
Здравствуйте, IT, Вы писали:

G>>Ну не совсем соглашусь. LSP нарушается, и у меня на памяти свежи такие нарушения, даже там, где есть очень простая иерархия: один абстрактный класс или интерфейс и всего 2-3 реализации.


IT>Это как раз и есть подтверждение моих слов. Если тебе или кому-то на твоей памяти удалось такое спроектировать, то проблема вовсе не в нарушении LSP. Наверняка там ещё то порождение "творца".


Не, там нормально всё было. Грубо говоря, некий построитель отчетов (абстрактный класс), который разные виды отчетов генерит (csv, pdf, word и т.д.). И просто в эту иерархию потом засунули совсем другой класс-потомок, который своим чуждым поведением нарушил стройную картину "абстрактной логики", внеся в тот слой явную обработку вариаций этого "особого поведения" подкласса-бастарда. А это явное нарушение LSP. В итоге просто мы поняли, что этот класс-потомок вообще не относится к данной иерархии и вынесли его в отдельное место. Иерархия после этого сказала нам спасибо и восстановила принципы LSP и DIP.
www.nanonewsnet.ru
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.