Здравствуйте, #John, Вы писали:
J>Как вежливо попросить что-то переписать, потому что код выглядит не очень? J>"The code will be look much better if ..., what do you think?"
А официально принятые в рамках организации или проекта гайдлайны по коду есть?
Здравствуйте, #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 тексты? Не лучше ли на дейли всех хвалить за их работу, и говорить что код ревью — это совместная работа и т.д., что все молодцы, что замечания на код ревью не призваны никого оскорбить или наругать. А в замечаниях просто и лаконично писать по делу?
Здравствуйте, #John, Вы писали:
J>как вежливо делать замечания по коду
1. делать вежливо. Ну в смысле не говорить, что это вот как-то по-мудацки написано, а говорить, ну, напирмер, что этот фрагмент сложный для понимания
2. не делать замечания не относящиеся к коду. Т.е. если бы ты написал этот код иначе, чем написал автор, но написанное автором решает задачу, укладывается в требования и гайдлайны компании, если они есть, и в целом решение не хуже чем то, которое предпочел бы ты — то тут и не место замечаниям.
J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?
Указать конкретные проблемы в чем "не очень", если они требуют переписывания.
Не забывай, что все эти переписывания — это время и силы. Их нужно тратить, если от этого улучшается код и не нужно иначе. Соответственно — следует обосновать, зачем человеку тратить на это силы.
Здравствуйте, fmiracle, Вы писали:
J>>как вежливо делать замечания по коду
F>1. делать вежливо. Ну в смысле не говорить, что это вот как-то по-мудацки написано, а говорить, ну, напирмер, что этот фрагмент сложный для понимания F>2. не делать замечания не относящиеся к коду. Т.е. если бы ты написал этот код иначе, чем написал автор, но написанное автором решает задачу, укладывается в требования и гайдлайны компании, если они есть, и в целом решение не хуже чем то, которое предпочел бы ты — то тут и не место замечаниям.
J>>Как вежливо попросить что-то переписать, потому что код выглядит не очень?
F>Указать конкретные проблемы в чем "не очень", если они требуют переписывания. F>Не забывай, что все эти переписывания — это время и силы. Их нужно тратить, если от этого улучшается код и не нужно иначе. Соответственно — следует обосновать, зачем человеку тратить на это силы.
У команды должен быть чек-лист со сводом правил, и чтобы все разрабы были с ним согласны. Например, правило: применяем SOLID. Или: применяем clean code мартина и макконнела. Или: применяем GoF при необходимости. Испольщуем полиморфизм вместо switch в бизнес-логике. Работам по контрактоному варианту. Применяем DDD или же просто анемичную модель. И т.д. и т.п. Этот лист должен постоянно дополняться, чтобы отражать консенсус всей команды. В этом случае не придется постоянно дискутировать и аргументировать свою позицию, будет достаточно дать ссылку на нарушенное в чек-листе правило, а не писать мини-лекции в каждом замечании.
Вкратце — замечания лучше оформлять в виде вопросов. Не указали final — Shouldn't this be final? Переусложнили — Won't using xxx simplify this code? Забыли какой-то кейс — What about yyy?
Здравствуйте, gyraboo, Вы писали:
J>>>Как вежливо попросить что-то переписать, потому что код выглядит не очень? F>>Указать конкретные проблемы в чем "не очень", если они требуют переписывания. F>>Не забывай, что все эти переписывания — это время и силы. Их нужно тратить, если от этого улучшается код и не нужно иначе. Соответственно — следует обосновать, зачем человеку тратить на это силы.
G>У команды должен быть чек-лист со сводом правил, и чтобы все разрабы были с ним согласны. Например, правило: применяем SOLID. Или: применяем clean code мартина и макконнела. Или: применяем GoF при необходимости. Испольщуем полиморфизм вместо switch в бизнес-логике. Работам по контрактоному варианту. Применяем DDD или же просто анемичную модель. И т.д. и т.п. Этот лист должен постоянно дополняться, чтобы отражать консенсус всей команды. В этом случае не придется постоянно дискутировать и аргументировать свою позицию, будет достаточно дать ссылку на нарушенное в чек-листе правило, а не писать мини-лекции в каждом замечании.
0. Я полностью тут с тобой согласен
при этом
1. обращение к справочнику проблем и практик — это тоже вариант обоснования же
2. и оно все равно может порождать споры есть тут нарушение или нет и требовать прояснения ПОЧЕМУ тут на твой взгляд происходит нарушение. Ну, в смысле если принято писать в SOLID и автор знает, что так принято, и написал иначе — то значит или он вредитель или он считает что написал правильно и просто сказать "у тебя тут нарушение SOLID" без пояснений — мало что даст же
Здравствуйте, fmiracle, Вы писали:
J>>>>Как вежливо попросить что-то переписать, потому что код выглядит не очень? F>>>Указать конкретные проблемы в чем "не очень", если они требуют переписывания. F>>>Не забывай, что все эти переписывания — это время и силы. Их нужно тратить, если от этого улучшается код и не нужно иначе. Соответственно — следует обосновать, зачем человеку тратить на это силы.
G>>У команды должен быть чек-лист со сводом правил, и чтобы все разрабы были с ним согласны. Например, правило: применяем SOLID. Или: применяем clean code мартина и макконнела. Или: применяем GoF при необходимости. Испольщуем полиморфизм вместо switch в бизнес-логике. Работам по контрактоному варианту. Применяем DDD или же просто анемичную модель. И т.д. и т.п. Этот лист должен постоянно дополняться, чтобы отражать консенсус всей команды. В этом случае не придется постоянно дискутировать и аргументировать свою позицию, будет достаточно дать ссылку на нарушенное в чек-листе правило, а не писать мини-лекции в каждом замечании.
F>0. Я полностью тут с тобой согласен F>при этом F>1. обращение к справочнику проблем и практик — это тоже вариант обоснования же F>2. и оно все равно может порождать споры есть тут нарушение или нет и требовать прояснения ПОЧЕМУ тут на твой взгляд происходит нарушение. Ну, в смысле если принято писать в SOLID и автор знает, что так принято, и написал иначе — то значит или он вредитель или он считает что написал правильно и просто сказать "у тебя тут нарушение SOLID" без пояснений — мало что даст же
SOLID — достаточно объективный набор принципов. Их нарушения в 90% видны объективно. Проблемы с SOLID и споры обычно бывают из-за того, что кто-то не совсем корректно понимает некоторые принципы. Поэтому да, часто приходится не просто указывать на нарушение SOLID, но и читать мини-лекцию прямо в замечании. Это обычно для джунов и мидлов делается и является неплохим вариантом их обучения "на реальных примерах", и такие временные издержки должны конечно же закладываться в оценку сроков и эта практика должна являться явной и "официальной" частью культуры разработки, а не делаться изподтишка или абы как.
J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?
Можно спросить "почему ты решил именно скопировать эту часть кода, а не выделить функцию? Создал таск чтобы разобраться." вместо "з??;л уже копипастить! деклайн!"
Здравствуйте, #John, Вы писали:
J>Как вежливо попросить что-то переписать, потому что код выглядит не очень?
Напомнило мне как в почтовые письма автоматически вставляется текст, вроде с уважением ля ля ля. Хотя лично я такое не люблю так как по сути это ненужная для прочтения информация.
Здравствуйте, #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
Здравствуйте, gyraboo, Вы писали:
G>SOLID — достаточно объективный набор принципов. Их нарушения в 90% видны объективно. Проблемы с SOLID и споры обычно бывают из-за того, что кто-то не совсем корректно понимает некоторые принципы. Поэтому да, часто приходится не просто указывать на нарушение SOLID, но и читать мини-лекцию прямо в замечании. Это обычно для джунов и мидлов делается и является неплохим вариантом их обучения "на реальных примерах", и такие временные издержки должны конечно же закладываться в оценку сроков и эта практика должна являться явной и "официальной" частью культуры разработки, а не делаться изподтишка или абы как.
Только не забудь сказать джунам, что если дело дошло до применения SOLID, то в коде уже проблемы, не смотря на то будут или нет применены принципы SOLID
Если нам не помогут, то мы тоже никого не пощадим.
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?" вопрос уровня "наставник ученику", т.е. корректен если ты какой лид, или сеньёр ревьювящий за джуном. А для равных между собой он используется только когда вы уже враги которые друг друга валят при первой возможности. Злорадное "вот ты и попался! вот тут нот дан!".
Короче это надо спрашивать не тут, а на форуме нейтивных нейтивов, живущих офисной жизнью.
Здравствуйте, IT, Вы писали:
G>>SOLID — достаточно объективный набор принципов. Их нарушения в 90% видны объективно. Проблемы с SOLID и споры обычно бывают из-за того, что кто-то не совсем корректно понимает некоторые принципы. Поэтому да, часто приходится не просто указывать на нарушение SOLID, но и читать мини-лекцию прямо в замечании. Это обычно для джунов и мидлов делается и является неплохим вариантом их обучения "на реальных примерах", и такие временные издержки должны конечно же закладываться в оценку сроков и эта практика должна являться явной и "официальной" частью культуры разработки, а не делаться изподтишка или абы как.
IT>Только не забудь сказать джунам, что если дело дошло до применения SOLID, то в коде уже проблемы, не смотря на то будут или нет применены принципы SOLID
Почему? Не улавливаю мысль. Конкретный пример можешь привести, где после применения SOLID остаются проблемы?
Здравствуйте, gyraboo, Вы писали:
IT>>Только не забудь сказать джунам, что если дело дошло до применения SOLID, то в коде уже проблемы, не смотря на то будут или нет применены принципы SOLID G>Почему? Не улавливаю мысль. Конкретный пример можешь привести, где после применения SOLID остаются проблемы?
Не после применения, а до.
Возьмём тот же LSP, который гласит:
объекты в программе должны быть заменяемыми на экземпляры их подтипов без изменения правильности выполнения программы
Когда возможно нарушение этого принципа? Например, в случае если у нас имеется иерархия объектов с развесистым и запутанным интерфейсом наследования. Принцип предписывает быть внимательнее с такими иерархиями и наследованиями. Но если дело вообще не доводить до таких иерархий, то не потребуется и сам принцип.
Если нам не помогут, то мы тоже никого не пощадим.
Здравствуйте, IT, Вы писали:
IT>>>Только не забудь сказать джунам, что если дело дошло до применения SOLID, то в коде уже проблемы, не смотря на то будут или нет применены принципы SOLID G>>Почему? Не улавливаю мысль. Конкретный пример можешь привести, где после применения SOLID остаются проблемы?
IT>Не после применения, а до. IT>Возьмём тот же LSP, который гласит:
IT>
объекты в программе должны быть заменяемыми на экземпляры их подтипов без изменения правильности выполнения программы
IT>Когда возможно нарушение этого принципа? Например, в случае если у нас имеется иерархия объектов с развесистым и запутанным интерфейсом наследования. Принцип предписывает быть внимательнее с такими иерархиями и наследованиями. Но если дело вообще не доводить до таких иерархий, то не потребуется и сам принцип.
Ну не совсем соглашусь. LSP нарушается, и у меня на памяти свежи такие нарушения, даже там, где есть очень простая иерархия: один абстрактный класс или интерфейс и всего 2-3 реализации.
А насчет сложных иерархий — согласен. Ну на это уже есть и этой бочке затычка другие принципы: заменяйте наследование композицией, используйте магическое число 7 (а лучше меньше, как ограничение кратковременной памяти), KISS.
Здравствуйте, 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'. Крайне редко пишу жестко (если это не первая попытка ревью, а человек не понимает).
Здравствуйте, gyraboo, Вы писали:
G>Ну не совсем соглашусь. LSP нарушается, и у меня на памяти свежи такие нарушения, даже там, где есть очень простая иерархия: один абстрактный класс или интерфейс и всего 2-3 реализации.
Это как раз и есть подтверждение моих слов. Если тебе или кому-то на твоей памяти удалось такое спроектировать, то проблема вовсе не в нарушении LSP. Наверняка там ещё то порождение "творца".
Если нам не помогут, то мы тоже никого не пощадим.
Здравствуйте, IT, Вы писали:
G>>Ну не совсем соглашусь. LSP нарушается, и у меня на памяти свежи такие нарушения, даже там, где есть очень простая иерархия: один абстрактный класс или интерфейс и всего 2-3 реализации.
IT>Это как раз и есть подтверждение моих слов. Если тебе или кому-то на твоей памяти удалось такое спроектировать, то проблема вовсе не в нарушении LSP. Наверняка там ещё то порождение "творца".
Не, там нормально всё было. Грубо говоря, некий построитель отчетов (абстрактный класс), который разные виды отчетов генерит (csv, pdf, word и т.д.). И просто в эту иерархию потом засунули совсем другой класс-потомок, который своим чуждым поведением нарушил стройную картину "абстрактной логики", внеся в тот слой явную обработку вариаций этого "особого поведения" подкласса-бастарда. А это явное нарушение LSP. В итоге просто мы поняли, что этот класс-потомок вообще не относится к данной иерархии и вынесли его в отдельное место. Иерархия после этого сказала нам спасибо и восстановила принципы LSP и DIP.
Если нам не помогут, то мы тоже никого не пощадим.
а "Is this ... done?" вопрос уровня "наставник ученику", т.е. корректен если ты какой лид, или сеньёр ревьювящий за джуном. А для равных между собой он используется только когда вы уже враги которые друг друга валят при первой возможности. Злорадное "вот ты и попался! вот тут нот дан!".
