Спасибо соеру за ролик, часть с кодом выглядит очень интересно. Вот может это можно было б сделать отдельной рубрикой. В моменте, где ты обьяснял про «принцып единственной ответственности» Есть такие короткие видосы в вкладочке «short”. Рубрика типа «теория и практика» . Я бы с удовольствием смотрел бы. Типа кусок кода и сразу там какой то принцып. А этих принцыпов очень много. И можно точно по 2-3 на каждый с разной переодичностю делать. В общес в заключение скажу, что очень понравился момент с кодом и обьясненем почему жто хорошо или плохо согласно какому-то принцыпу. Спасибо за ролик!

Спасибо, очень информативно. К сожалению практики повсеместного ревью в компании нет, но когда был junior разработчиком, был ментор, который делал ревью и это действительно очень способствовало росту.

больше всего понравилось про "раздаем советы, но не даем приказы", по моему опыту за 5 лет, именно 90% выдает приказы. спасибо за видео, много полезного

@ThNIKolas

2 жыл бұрын

Советы - тот же приказ))) только вежливый, так как айтишнега низя абидить)))

@alexlightweight

2 жыл бұрын

потому что IT сфера стала очень токсичной за последние лет 5-6, снаружи это не очень видно и все по прежнему выглядит красиво и модно-молодёжно, но во многих конторах процветает совок и кумовство, при чем я про частные конторы, а не госшараги, отсюда и приход в IT отрасль "приказчиков", "крикунов" и "сроковыжимателей", ну и приправлено это все проходимцами-менеджерами, которые вчера были торгашами стройматериалов или каких то услуг, а теперь они рулят разработкой ПО. жиза ...

@SlavaCh

2 жыл бұрын

Ну да, мою ветку как и других разрабов смотрит сеньор и тупо не мерджит ее, пока я не закроют все замечания. Автократия в программировании)

@user-ve7kd3mt2g

Жыл бұрын

@@alexlightweight да, но в отличие от других сфер, где происходит тоже самое, в айти хотя бы за это платят.

@alexlightweight

Жыл бұрын

@@user-ve7kd3mt2g да, согласен, но в других сферах хотя бы не делают вид что у них все "модно-молодежно", то есть такого лицемерия типа "это в газпромах все по блату и через связи, а у нас в IT этого ничего нет", хотя по мне в IT сейчас уже почти как в газпроме, особенно в плане всяких менеджеров и прочей около айтишной шушеры )))

Спасибо за хорошие темы! Продолжай дальше!

Супер! Очень важная тема и было интересно послушать

Спасибо за классные советы "как проводить"

Очень хотелось бы видео о работе драйвера изнутри, и о его написании, видео на эту тему в русском сегменте очень мало, буду очень благодарен !!!)))

@lgotindev6712

2 жыл бұрын

Поддерживаю!

@user-xq2ng2cu3d

2 жыл бұрын

Вооьще конечно не в тему, но да было бы интересно)

вот это чоткое видео! благодарочка вам от вебмастера и сеошника со стажем СЕООНЛИ

Хорошее видео, тут где-то перекликаются советы с тем, что говорил Егор Бугаенко. Я думаю это здорово, если такие люди как вы рассказывают и доносят внятно такую информацию, значит можно внести это в команду

спасибо понравилось все очень круто !

Спасибо за интересное видео. А что, если у нас команда из 5 фронтендщиков, но каждый работает в отдельном репозитории на своём проекте в одиночку, и верстает и если надо прикручивает React/Vue? У нас типа digital agency. Есть ли смысл делать код ревью проектов, в которых ты ничего знать не должен и за которые ты не отвечаешь перед менеджером? Как просить на это время?

Спасибо за видео, Евгений! Все-таки я не стал внедрять кросс-ревью в больших командах, например, 10+ разработчиков))

@S0ERDEVS

2 жыл бұрын

Большие команды все равно должны биться на группы, 3-5 человек. В рамках группы делаться ревью.

@hakooplayplay3212

2 жыл бұрын

10+ это уже не команда а целый отдел

Оч крутая тема и описание ) спасибо ) беда в том что многие этого всего не понимают ) и код ревью превращается в холивары ) без смысла )

В коде, где ничего не надо менять, я бы фор на вайл тру поменял, последняя конструкция лучше в глаза бросается. Но это так, вкусовщина.

Звук вроде поменялся?

"как только вы начинаете много тратить времени на комментарии к ревью, эта ветка должна блокироваться и ... выноситься на обсуждение." забавная рекурсия)) а вообще, материал годный - спасибо автору

Допускаешь ли ты дописывание/переписывание/исправление/улучшайзинг/рефакторинг кода ревьювером в отдельных случаях? Если да то в каких, и в каком обьеме ( как это вообще измерить или формализовать )

@S0ERDEVS

2 жыл бұрын

Нет, тогда нужно использовать парное программирование, а не ревью.

like!

да, хорошее ревью не должно быть долгим :-)

Вопрос: какое минимальное кол-во ревьюеров у вас стоит на PR?

Жаль нет важной для меня информации - как распределять мерж реквесты на апруверов. Есть ли какие-то примеры автоматического назначения или кто-то должен вручную выбирать.

Добрый день. Вопрос про "Ревью - это советы а не приказы". Что делать если коллега или подчиненный вообще не старается расти в профессиональном плане? Например за год человек по уровню кода не вырос вообще, зачастую забивает на корпоративные стандарты и так далее. Изначально это было в форме советов. Но через какое-то время перешло на прямые приказы. Понятно, что частично проблема кроется в стиле управления и наставничества и возможно стоит что-то поменять) Но и люди тоже не всегда хотят развиваться - для кого то это просто работа, выполнение функции.

@user_gennadii

Ай бұрын

Как по мне эта проблема выходит за рамки ревью кода. У нас например в компании есть люди, которые пишут нечитабельный код и ничего с этим не поделать. И уволить жалко, и ревьюить код невозможно

Спросите у Сакутина, он об этом все знает 😂 Ну а вообще, видос топ, а code review абсолютно любому программисту на все стадиях обучения полезен

Когда-то работал в конторе, никакого кодревью. Работа в темпе «херак херак и в продакшн». Начальник заходил в офис со стандартной фразой «ну чо там, когда готово будет?». Деньги от государства, уже выделены, результат лишь бы был.

@kulikoffAS

2 жыл бұрын

через какой срок в итоге такой проект стал сыпаться и стал неподдерживаемым?

тот момент, когда ты несколько недель просил поревьюить свой код, а в итоге теперь им нужно "ревьюить" фичу в которой изменено 6000 строк =)

Что делать в ситуации, когда ревьювер - опытный разработчик, который не хочет пропускать плохой код? Имеет ли смысл все таки настаивать на хорошем решении на ревью, чтобы плохие решения не просачивались в кодовую базу?

@hakooplayplay3212

2 жыл бұрын

Если все плохо но выходит спор, думаю имеет смысл привлечь третью сторону

@S0ERDEVS

2 жыл бұрын

Вопрос не совсем корректный, мы не можем знать заранее внесение изменений в код улучшит, ухудшит или не изменит качество кода, поэтому должны рассмотреть три случая: - что делать если ревьюер дает плохое замечание и автор кода должен его исправить? - что делать если ревьюер дает хорошее замечание и автор кода должен его исправить? - что делать если ревьюер дает нейтральное замечание и автор кода должен его исправить? если наступление всех вариантов равновероятно, то получается в двух случаях мы делаем лишнюю работу которая как минимум не улучшает ситуацию, и только в одном случае делаем работу которая может помочь. По своим проектам я регулярно смотрю какие замечания даются на ревью, около 10% действительно замечания, которые очевидно делают код лучше (с ними авторы кода и не спорят), остальное - вкусовщина (т.е. реально нельзя проверить улучшился код после устранения замечания или нет, только субъективное ощущение у автора ревью). Думаю совершенно очевидно, что в тех редких случаях когда ревьюер дал хорошее замечание, а автор кода его не принял, можно спокойно обсудить это на дейлике, и не тратить часы на эмитацию бурной деятельности, делая замечания, которые тормозят проект и заставляют компанию тратить деньги впустую. Поэтому достаточно править только те части где и автор кода, и ревьюер легко пришли к согласию, что поправить стоит - это и есть те вещи, которые очевидны. В остальных случаях особого смысла заставлять нет.

Для меня странно звучат 3 момента: 1. кодревью - это совет. 2. кодревью - это оценка кода, а не автора. 3. хороший код не обязательно понятен человеку с улицы. Первый звучит странно потому, что если ты несёшь ответственность за проект и в твоей команде есть человек, который пишет плохой код и отказывается его исправлять, то оставить этот код в ответственности этого человека означает взять на себя риск. Этот человек через год, когда наделает кучу гадостей в коде, может свалить в другую компанию, оставив своих коллег, а главное тебя самого, ведь ответственность несёшь ты, это всё разгребать (и да, у меня такое было один раз). А главное ты это можешь узнать ещё через полгода после того, как он уйдёт, а тебе прилетит задача, связанная с написанным им кодом, и ты поймёшь, что его поддерживать невозможно. Второй звучит странно потому, что когда человек из раза в раз совершает одни и те же ошибки, особенно если они очевидны, то говорить ему "и тут поправь тоже" становится слегка бессмысленно. В этот момент надо переходить именно к оценке автора кода, узнавать его мотивацию так писать код, попытаться понять не является ли эта ошибка результатом пробела в знаниях, который надо заполнить и т.д. То есть код, однажды написанный плохо - это плохой код, а код, который систематически пишется плохо - это уже свидетельствует о плохом авторе кода. Третий пункт я считаю странным потому, что это как сказать "речь сотрудников компании не обязана быть понятной человеку с улицы". То есть представьте себе ситуацию, когда в русскоговорящую компанию берут русскоговорящего человека, он выходит в первый день на работу и обнаруживает, что речь его коллег звучит как "правти! Ям сидня зончу задачу вобедень". Я не думаю что тут было бы уместно говорить "а, так это нормально, у нас так принято в компании, через месяцок-другой освоишься" :) Так почему же это нормально в отношении кода тогда? И да, в этом отношении второй код из примера лучше первого, потому что хоть в нём и есть некоторые упущения, но его может легко прочитать человек с улицы, типа меня (инициализируются переменные, парсится переданное состояние таймера с получением значений старта и интервала таймера, эти значения из миллисекунд переводятся в секунды+наносекунды, они в свою очередь собираются в объект "спецификация таймера", после чего эта спецификация применяется к таймеру). Первый код я скорее всего прочитать бы смог, если бы погрузился в тему и язык, который знаю посредственно, но в том то и дело, что второй код легко понимается при беглом просмотре, а первый можно понять только погрузившись, а значит он по определению сложнее читается :)) Есть предположение, что в первом коде не хватает уровня абстракции, скрывающей ненужные детали низкого уровня, но это не точно...

@theway3305

2 жыл бұрын

присоединяюсь, а про первый пункт особенно, тоже вопрос возник, если в команде кто-то "говнокодит" или опыта мало, это все мержить без возражений? что тогда с проектом станет? наверное, если команда состоит из таких людей, как автора канала, то да, может и можно советом ограничиться.

@S0ERDEVS

2 жыл бұрын

1. Вы описываете стандартную проблему, когда менеджмент экономит на найме и хочет чтобы в плохой команде один хороший программист все разгребал. Это не работает. Решайте вопрос с наймом, тогда не надо будет бегать за каждым разрабом и вытирать ему сопли. Куда вы смотрели когда принимали человека на работу? Если он плохо работает, то используйте механизм депремирования. Причем тут ревью кода? 2. То же самое что первый пункт, решайте вопрос с наймом. Ревью тут не при чем. Раз наняли - то работайте с человеком. используйте парное программирование, повышение квалификации, кнут и пряник. Но сваливать косяки плохого работника на "ответственного" за код - это глупо. В итоге человек уйдет туда где будет более здоровый климат в коллективе. 3. Речь идет о бизнес-логике приложения. Почти везде есть либо термины, либо сокращения, которые будут непонятны человеку с улицы. Поэтому если кто-то не знает что такое ЦККИ, ЦОС, ЦУИ, АС ЭКР - это вполне нормально, полное описание этих сокращений неуместно в коде. Тоже касается архитектурных ограничений и требований, они не обязаны быть одинаковыми. И много других нюансов.

@nikolaymatveychuk6145

2 жыл бұрын

@@S0ERDEVS 1. Возможно, но это не отменяет того. что плохой код ДОЛЖЕН быть исправлен, а не рекомендуется к исправлению. Я понимаю о чём Вы, и Вы конечно же правы в отношении "нанимайте нормальных сотрудников сразу", но это описание идеальной ситуации, а в жизни бывает разное, начиная от недосмотра при найме (например не видит сотрудник разницы между переменными orderId и idOrder, и на собеседовании это не проявилось) и заканчивая банальной попыткой вписаться в бюджеты получив приемлемый результат :) 2. Ответственных назначают же на проекты не ради красивых титулов. Да и даже для меня, как разработчика ответ "я знаю, что я сеньёр, а он миддл... но я ему говорил, что надо исправить, а он отказался, что я могу с этим сделать?" звучит слегка странно, а для руководства вообще будет из разряда бреда. Если это заявление руководителя, то фиговый он руководитель, раз не убедил своего подчинённого исправить код, а если он просто более опытный коллега, то зная, что в коде есть данная ошибка, должен был донести информацию до того, кто на это может повлиять. 3. По поводу сокращений - надо понять реально ли они нужны. Например часто в контексте полное название переменной не нужно, и какой-нить vat можно легко заменить на tax. Я могу предположить случаи, когда от сокращения уйти нельзя именно по причине того, что это может внести неоднозначность в код, но это не происходит в коде повсеместно, потому скорее исключение, чем правило.

@S0ERDEVS

2 жыл бұрын

@@nikolaymatveychuk6145 1. не получится в вашей бизнес-модели построить хороший код. Вы даже в первом своем комментарии обозначили, что в такой модели вы будете постоянно пропускать плохой код и лишь спустя время случайно натыкаться на него. В данном случае вы пытаетесь за счет ревью решить вопрос квалификации, поэтому и возникают всякие дикие артефакты "контроля". Работать такая модель будет, но думаю текучка будет очень большой. Работать в таком проекте лично я бы не хотел. 2. Это уже система мотивации, а не построение кодовой базы. У меня не было в практике, чтобы разраб был согласен, что код плохой но не хотел его исправлять. В конце концов у них премия зависит от того какая оценка их работы будет сделана руководством. Ну это бредово на предложение "давай избавимся от дублирования кода и используем стандартную функцию" человек ответил "нет я не хочу ничего исправлять, я хочу чтобы код было максимально ужасным". А вот если сказать "мне не нравится как ты назвал переменную X, я хочу чтобы ты назвал ее Y", то здесь будет много споров, ибо вкусовщина. И держать проект из-за того, что два упертых челика не могут договориться о имени переменной - это бред. 3. Есть не только сокращения. например есть "банковское сокращение" и "математическое" для человека который привык к математическому будет "куча ошибок в коде" который работает с банковским округлением. Есть даже отдельный порядок округления, который должен точно выражаться в коде. И так в любой области.

@S0ERDEVS

2 жыл бұрын

@@nikolaymatveychuk6145 вообще взгляд на ревью сильно зависит от структуры и политики компании, если покопаться в комментах канала, то несколько лет назад я отстаивал позицию по ревью очень похожую на Вашу и это сильно зависело от того, с кем я работал и какие требования были ко мне. Сменил работу, почувствовал другие подходы и мне сейчас больше нравится именно такой подход как в видео, и выстраивание команды от "одноранговости" и "вовлеченности". Так что тут сильно играет то того с кем работать.

8:25 но по какой-то причине люди до сих пор не используют linter'ы и прочие штуки для стандартизации форматирования кода на проекте =\

Хорошее видео, но, на мой взгляд, остался непокрытым вопрос о том, насколько ревьювер кода должен пытаться вникнуть именно в правильность выполнения бизнес требований по конкретной задаче, а не только оценки качества кода и выявления потенциально проблемных мест - это сродни запуску кода и тестированию, или всё-таки имеет смысл уделять этому особое внимание. Бизнес требования могут иметь достаточно высокую сложность, было бы интересно мнение автора исходя из его опыта, ведь в таком случае ревью уже может выйти за рамки быстрого 5-10-минутного просмотра.

@tesgreat8958

2 жыл бұрын

филосовский вопрос. кажется автор уже ответил пунктом о незапуске кода и тестов. правильность выполнения бизнес кейсов все таки я бы на ревью не оценивал (да и не хотел бы что бы оценивали меня). Может только явные какие то ошибки указать. Тем более если есть на проекте практика покрытия тестами

@S0ERDEVS

2 жыл бұрын

Человек должен пытаться понять все что он может понять в течение 10-15 минут, наметанный взгляд может заметить и некоторые баги, но как правило это все таки больше действия на уровне понимания/не понимания кода. Ревью должно помогать писать понятный код, корректный код должны помогать писать тесты, типы, верификация. Задачи разные.

@Ihor.Korolov

2 жыл бұрын

@@S0ERDEVS категорически не согласен. Если я потрачу час на ревью и найду ошибки в бизнес логике, это потом сэкономит кучу времени как для тестировщика, который не факт что еще заметит баг, так и для автора кода, которому придется переделывать и отправлять еще раз на ревью, в котором мне опять придется принять участие. Чем быстрее этот замкнутый круг разорвется - тем лучше.

@S0ERDEVS

2 жыл бұрын

@Ihor Korolov почему вы написали "если"? Написали бы "я каждый день трачу час на ревью кода коллег, нахожу Х багов, вношу Y корректив", но это утопия. Вы очень быстро устанете от такого напряженного ритма, и сгорите от мысли, что вы тратите кучу сил, а вашу работу считают "ну что такого, быстренько пробежался по коду". При этом "успешных" ревью кода будет найден действительной мажорный баг будет крайне мало, в основном это будет рутина, которая вытянет все силы, и на свою работу ничего не остаентся. И если вы не можете измерить и сказать сколько это "куча времени", то это никому не интересно, то что вы будете считать "кучей" остальные посчитают "ничем".

@AlexTkachiov

2 жыл бұрын

@@S0ERDEVS особенно, если у тебя в команде хотя бы 4-5 человек, которые в хорошем ритме закрывают задачи, и ты при этом лид, например. При таком подходе достаточно быстро поймешь, что кроме детального ревью пул-реквестов, тупо нет времени на все остальное.

@SOER я не совсем понял вашу мысль относительно ответственночти за код. Кроме программиста над кодом работвют тестировщики, тим лид, другие члены команды. Если код утверждается ими всеми и попадает в продакшен, то, по моему, за него должен отвечать не только программист его написавший. Разве не так ?

@S0ERDEVS

2 жыл бұрын

Смешали все в кучу. Тестировщик не за код отвечает, а за тестирование, а тимлид отвечает за организацию работы. Ответственность разработчика - это не только Ответственность за баги, но и за скорость, сопровождаемость и другие характеристики.

@MrAndycount

2 жыл бұрын

@@S0ERDEVS думаю мы говорим о разных вещах. Мой поинт в том, что зона ответственности одного начинается там, где заканчивается другого. Не может один человек отвечать пожизненно за свою работу, если куча других людей приняла его работу. Разве не так ? Может я неправильно вас понял и, говоря про ответственность программиста, вы имеете в виду только время от получения звдачи до её закрытия ? И ещё воарос. В одном выпуске вы говорили про боязнь совершить ошибку, что этого не нужно бояться. И я с вами согласен, так как ошибки в коде - это неотъемлемая часть разработки Но в этом выпуске, как мне показвлось, вы противоречите сами себе. .Не будет ли программист бояться закрывать свои звдачи, если от него будут требовать слишком тщательной проверки своего кода перед пуш реквестом ? Не будет ли в этом вреда проекту ?

@S0ERDEVS

2 жыл бұрын

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

@MrAndycount

2 жыл бұрын

@@S0ERDEVS я вас понял. Вы говорите не про ответственность, а про газлайтинг и абьюзинг в коллективе. Думаю, это тема для психологов.

Похоже что описана ситуация команды из примерно равных по скилу людей. Но если у вас команда джунов, наверное все же скидывать на джуна 100% ответственность за функциональность или делать ревью в виде советов, как то не очень сработает?

@S0ERDEVS

2 жыл бұрын

Тогда используйте парное программирование. Если у вас один хороший разраб и 10 джунов, то ревью вам все равно ничего не даст.

А если есть Code Style Guideline и код ему не соответствует, при этом автор на указание об этом в комментах отвечает отказом что-то менять, тут надо оставить право принятия решения за автором? А если код не выполняет (или выполняет с ошибкой) требуемой функциональности? Тоже решает автор? Мне правда интересно, это не попытка обидеть. Про право Вета не упомянуто, странно. Хотя в сервисах code review такое есть.

@tesgreat8958

2 жыл бұрын

про код стайл тут наверное был пункт про автоматизацию. обычно большинство проверок по код стайлу можно автоматизировать. Насчет ошибок - ну если это по коду сразу видно, то конечно нужно указать на ошибку. Но скорее тут нужно бы указать на отсутствие тестов)

@S0ERDEVS

2 жыл бұрын

Тогда линтер не пустит этот коммит в ветку. На ревью должен попадать код: 1. Прошедший проверку статическим анализатором (вот его замечания обязательны) 2. Прошедший тестирование разработчиком (через тесты или вручную) 3. Прошедший локальную сборку у разработчика на окружении аналогичном продкутовому У вас пункты 1-2 явно нарушаются. Я считаю, что право "вето" невозможно в здоровой команде. Если человек откровенно вредит проекту, обсуждайте это на дейлике, а не в ревью.

@viktortarasov2142

2 жыл бұрын

За ответ большое спасибо, но... Линтер, тесты, build server... У меня даже code review в проекте нет :) При наличии как минимум трёх разработчиков моей сферы/языка, считая меня. Видимо, мне давно пора бежать в другой проект. Верю, что в других проектах чаще встретишь наличие code review при отсутствии проверки линтером, статическим анализатором.

@S0ERDEVS

2 жыл бұрын

Если будете расти, то все появится. Это же все атрибуты зрелости проекта.

Пересматриваю контент и понимаю, что основы программирования следовало бы начинать преподавать как можно раньше.. Последовательность и логичность у людей стали бы чаще встречаться 😄😅 если бы программисты пришли к власти, мир бы разительно изменился 🤔

"Коды не пахнут" - А. Капоне

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

Моя практика показывает что все код ревью приводят к срачам и обидам, но код не особенно то и улучшают. Мне кажется что если человек привык к бардаку по жизни, то есть дома бардак, вещи раскиданы, куча хлама везде валяется, то код он будет писать точно так же и его не переделаешь, к сожалению. Заставить на время можно, но переделать его не получится все равно. Но это так, наблюдения из жизни. )))

@evgenyamorozov

2 жыл бұрын

Поэтому я решил, что я обязан принять чужой код, если в нём нет багов. Мне что-то может не нравится, я могу предложить переименовать что-то, добавить/убрать пару классов. Но если там нет багов - я обязан заапрувить. Именно обязан. И уже автор кода решит, хочет он что-то менять согласно советам или зальёт, как есть.

@alexlightweight

2 жыл бұрын

@@evgenyamorozov таки и должно быть, но на практике это сплошные понты в стиле "есть два мнения: мое и неправильное" ))) Если вы так делаете, то хорошо, но в основном делают как я описал выше

28+((0x3BBEECC>>2*month)&3) 16-тиричное число не верно указано, и эта формула не работает для високосного года можно сделать как 28+((( 0x3BBEECC+( 0x10* (1>>(year%4)) ) )>>2*i)&3) Скобки я сделал для приоритетности верной, тк не помню точно у каких операторов приоритет выше Возможно это можно лучше оптимизировать, код я писал для js Вот полный код для проверки: let day = 0; let year = 2005; for(let month=1;i>(year%4))))>>2*month)&3); console.log(month, day); }

сколько ж разрабов пересралось на этой почве…. Но вещь обязательная, хоть и может приносить боль, как на текущем проекте. Лид ревьют код в лучшем случае раз в неделю и он полностью солидарен с автором в вопросах типа ‘Why do we need this, what will be if, pls investigate etc” и так под каждой строкой, по факту имеем трудозатраты на такие ревью как 50% фичи. Сегодня было ревью пула который висел 4ю неделю и на очередную порция таких вопросов я понял что я просто банально не помню! Благо после брейншторма вспомнил что так заказчик на одном из дейли сказал, повезло. Касательно не запускать тесты и руками тыкать - согласен, но бывают исключения как опять таки на этом проекте - разраб делает фиксы не проверяя, то есть код написал и в пул, открываешь страницу а там 500, новый опыт для меня короче . А так в принципе всё описано как должно быть в идеальном мире, только вот есть ли он……,,

@tesgreat8958

2 жыл бұрын

ну ок. 500 - прилетит ему баг, поправит. с этим пунктом я как раз согласен на 100%. Есть тесты, есть CI, формально они должны показать что все как минимум собирается и тесты проходят. Дальше дело за тестированием. Ревью все таки не об этом

@tesgreat8958

2 жыл бұрын

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

@AlexTkachiov

2 жыл бұрын

Ревью пул реквеста 4 недели это уже нонсенс по многим причинам, что-то фундаментально поломано в процессе CI. Излишняя придирчивость тут уже как вишенка на торте просто.

@S0ERDEVS

2 жыл бұрын

Жесть какая.

@oeaoo

2 жыл бұрын

Срутся - значит не работают, а самоутверждаются.

Всегда делаю ревью собственного кода через пару-тройку дней после написание! И всегда возмущаюсь какой же идиот это писал!

@tesgreat8958

2 жыл бұрын

наказываете как то?

Спасибо за интересное видео. Не согласен, что не надо запускать код. Если проект сложный (например, компилятор на миллионы строк), то баги не всегда очевидны. Лучше дебагать и проверять перед тем, как писать замечание, иначе будет много неверных замечаний.

@PowWowVideo

2 жыл бұрын

+1, обязательно надо запускать код

@user-zk3uo3ti6f

Жыл бұрын

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

У меня на проекте постоянно брали джунов. Никаких анализаторов кода не стояло и код ревью никто не делал. Каждый был загружен своими задачами и писал как хотел. После того как у меня появлялось время на код ревью я хватался за голову от увиденного. Человека увольняли, а я всё переписывал. Другие приходили такие же. Все мои советы по написанию кода в большинстве случаев игнорировались, а у меня опускались руки. В итоге проект превратился в кучу сложно поддерживаемого говнокода. Если бы изначально был style guide, статические анализаторы и код ревью, то этого всего можно было избежать. Если сейчас на код ревью я будут просто что-то советовать, а не требовать то джуны просто напросто будут продолжать плодить говнокод. p.s. к сожалению при найме разработчиков моё мнение не учитывалось. Даже когда я лично собеседовал человека и говорил начальнику, что кандидат очень слабый, то он его всё равно брал в команду.

@S0ERDEVS

2 жыл бұрын

Это история из серии - водитель авто должен еще и в автомеханнике разбираться. В советское время автовладельцы сами чинили свои авто, по многим причинам, но основное - качество автомобиля было таким, что он постоянно ломался, и его надо было чинить. Сейчас наоборот водители не понимают и не должны понимать в автомеханнике. И это правильно! Моя аналогия в том, что никакое ревью не поможет если изначально вы ошиблись в кадровой политике и у вас нет качественных кадров. И да, можно добиться чего-то более менее работающего, но в конечном итоге вы просто устаните. Я видел такие команды, в какой-то момент программист, который тянул все это дело, уходит и проект разваливается.

@Photoshopper2010

2 жыл бұрын

@@S0ERDEVS вы правы. Всё к этому идёт

на счёта кол-ва нулей, то вместо 1000000 можно было просто 1e6 написать.

Напишу великую банальность. Везде по-разному, смотрите на свою ситуацию и вырабатывайте правила, наилучшим образом работающие именно для вашей команды.

Камера что-ли новая? Картинка ярче стала

@zhimbura

2 жыл бұрын

Там кстати где перевод времени, if не нужен результат не изменится

А, и еще. В какой этап ревью включить доказательство своему лиду, например, что код работает как надо? Код может быть внешне красивым и проходящим ревью, но не работать правильно. Как у нас сейчас: Если лид не уверен, что все работает как надо, то он должен иметь возможность запустить код с включенным дебаг аутпутом, например и посмотреть, как все работает, запустить тесты, провести автотесты и т.д. Неужели это уже не является этапом код ревью? Кажется, что пока ты находишься в контексте чужого кода, это самое логично - перейти к его проверке разными инструментами, по необходимости.

@S0ERDEVS

2 жыл бұрын

Тогда вам нужно парное программирование.

В свое время так и не решили задачу на проекте - код вроде хороший, ревью проходит, а бизнес кейсы не проходит. Баг на баге. Начинаем фиксить - получаются костыли, зато бизнес проходит. на повторное ревью нет времени, выходит замкнутый круг

@ThNIKolas

2 жыл бұрын

Это, конечно, офигительно, но есть же тесты))) Зачем кто-то разработал то, что не проходит свои же тесты?)))

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

На мой взгляд, тим лид все таки несет ответственен за то, что написал программист в его команде. Если происходит апрув, то ответственность также разделяется с лидом. В этом есть смысл, так как нельзя просто апрувить пул реквест для галочки и думать, что потом программист если что разберется. Каждая итерация с багом (или его возвращение) увеличивает время разработки. Лид должен препятствовать таким решениям, если нужно даже созвониться и обсудить пул, сделать checkout ветки и тд.

@S0ERDEVS

2 жыл бұрын

Нет, смысл ревью не в том чтобы разделить ответственность, а в том, чтобы оздоровить процесс разработки. Отлавливать баги должны тесты или верификация, заменить ревьюером тесты - это плохая идея.

@JohnDoe-ji1zv

2 жыл бұрын

@@S0ERDEVS возможно зависит от компании, у нас все очень строго и апрув просто так не получишь, соответственно качество кода очень высокое. Так же работал и там где апрув ставили просто советчики и качество было заметно ниже.

Я инженер программного обеспечения, но я не хочу быть Соером) Аббревиатура звучит так себе. Одного Соера хватит)

@S0ERDEVS

2 жыл бұрын

Предположу что имелось в виду не "быть", а "называться". Или все же быть?

Так, вот что я не понял. Допустим, разработчик у другого разработчика видит не замечание по чему-то, а ошибку, явную. И говорит об этом. У автора кода все еще есть возможность это игнорировать? Не рассматриваю причины, но если он продолжает настаивать, что ошибки нет - то где и как в процессе код ревью подключается арбитр? Например, это могут быть - тимлид, архитектор, или старший разработчик. Кажется, эти люди имею право сказать "надо переделать" и не подавать это в виде совета

@S0ERDEVS

2 жыл бұрын

Я не отношусь к ревью как к "поиску истины" или "виноватых". Вы нашли ошибку, вы указали на это разрабу, он посчитал исправление ненужным (например, ошибка очень редкая, а ее исправление стоит миллион рублей). Он имеет полное право отклонить ваш совет. Далее вы можете завести тикет с этой проблемой и он пойдет по стандартному процессу работы с кодом.

А нам тимлид говорил, что если апрувишь, значит одобряешь со всеми вытекающими. И он тоже крут, только канал не ведёт в ютубе. Кому верить?

@tesgreat8958

2 жыл бұрын

без канала на ютубе я бы не доверял)

@nikolaymatveychuk6145

2 жыл бұрын

тимлид прав )) Если сотрудник это делает не в своё свободное время, то значит должен нести за это какую-то ответственность. Иначе смысл в таком ревью? типа "мне щас 4 часа времени компания оплатит, а я посижу поиграюсь, а потом скажу, что ревью провёл". У ревью должен быть какой-то видимый результат, или в виде исправлений кода, или в виде подтверждения качества написанного кода.

@S0ERDEVS

2 жыл бұрын

Вообще вам виднее насколько у вас здоровая атмосфера в команде. Интересно какие "вытекающие" могут быть из ревью кода?

@S0ERDEVS

2 жыл бұрын

@@nikolaymatveychuk6145 а еще можно бигдату натравить на сотрудника )

@nikolaymatveychuk6145

2 жыл бұрын

@@S0ERDEVS если это входит в его компетенции и он готов этим заниматься (согласился при найме), то почему бы и нет? :) я же не говорю, что программиста надо заставлять шкафы собирать и бить молоком по рукам при косяках сборки. Он же выполняет ту работу, на которую нанимался - пишет код и поддерживает уже написанный (в том числе анализирует и исправляет найденные проблемы в нём).

Интересно что в видео такой подход к код ревью предполагается как единственно правильный, хотя это вообще не так. Отедельно порадовало "ревью это как совет в курилке, а не отдельный этап в задаче" и "ревью должно включать всю команду". Кто-то без опыта в код ревью может посмотреть и принять за чистую монету, но это вообще не так. Более того из опыта это ещё и не самый оптимальный вариант имхо. Все пункты перечисленные в самом начале, работают на 100% только если ревьювер полностью погружается в задачу и понимает каждую новую строчку кода (зачем она, почему так и т.д.). В таком подходе невозможно вовлечение всей команды, потому что это слишком затратно по времени, и не даёт большого преимущества (код вряд ли станет сильно лучше и знаний у автора не сильно прибавится от лишних ревьюверов). Здесь скорее всего получается противоречение из-за того, что пункты в начале расставлены не в правильном порядке. Основные плюсы ревью это именно улучшение кодовой базы и взаимное обучение коллег. "Эффект наблюдателя" вообще говоря довольно спорный, потому что он имеет минусы. Например порой когда автор знает, что его код проверят, теряется часть ответственности за свой код. Ты знаешь что его посмотрят и потенциальные баги найдут, поэтому сам можешь что-то недосмотреть. Нужно понимать что процесс код ревью довольно сильно зависит от специфики задач. Если каждая таска разбита на 5 модулей и включает кучу бизнес логики, то поверхностный просмотр ревью вообще невозможен, он просто ничего не даст из-за того, что будет абсолютно не понятно что происходит, пока ты не поймёшь задачу и решение. В то же время если обычно задачи в команде это минорные баги, то подход может полностью изменится

@S0ERDEVS

2 жыл бұрын

Если "эффект наблюдателя" не работает, то можете хоть костьми лечь, а улучшение кодовой базы не будет. Это основное что дает ревью, остальное это просто вишенка на торте. Можете посмотреть на историю комментариев к ревью в вашем текущем проекте, уверен там уровень замечаний "уменьши вложенность", "поменяй название переменной", "отрефактори метод". Я много слышу как на ревью "можно" отловить баги, на самом деле я подбивал статистику по своим проектам, там мизерный процент багов найдено в рамках ревью и то скорее случайно. Все становится гораздо лучше, когда для прохождение ревью нужна не виза конкретного человека, а чтобы пара человек из команды посмотрели и апрувнули. Если из кода непонятно что он делает, для людей, которые погружены в задачу, то это просто плохой код. Завтра разраб, который его делал, уволится или уйдет в отпуск и вы будете чесать репу думаю "а что он тут сделал".

@tramfromgames3595

2 жыл бұрын

​@@S0ERDEVS >на самом деле я подбивал статистику по своим проектам, там мизерный процент багов найдено Ну так это и подтверждает мой аргумент, что с таким подходом к код ревью баги не выявляются, потому что "10 минут пробежаться глазами" не способствует этому. >Если из кода непонятно что он делает, для людей, которые погружены в задачу, то это просто плохой код С чего бы вдруг случайный коллега делающий поверхностное ревью кода будет погружён в задачу. Порой в задачу не погружён никто помимо автора, такие задачи тоже бывают >Это основное что дает ревью, остальное это просто вишенка на торте Это просто утверждение без аргументации, я уже даже успел сказать почему это двоякая ситуация

@S0ERDEVS

2 жыл бұрын

@TramFromGames 1) тогда дайте статистику по своим проектам, будет интересно узнать сколько багов вы отловили на ревью. 2) не случайный, а коллега работающий в той же группе, над той же частью проекта. Случайный - это как раз мой условный "человек с улицы". 3) Вы какие-то странные штуки рассказываете, что значит "в задачу погружен один человек"? Т.е. он не ходит в отпуск, не болеет, у него нет коллег с которыми он обсуждает решение, выдвигает гипотезы? Что-то тут не сходится у вас. Похоже вы придумываете коллизию моим утверждениям, а не встречали реальную ситуацию, которую описываете. 4) если в задачу не погружен никто кроме автора, то и ревью не может никто сделать.

@tramfromgames3595

2 жыл бұрын

@@S0ERDEVS 1) Статистика сложная штука, разумеется я не занимался исследованием этого вопроса и не структурировал опыт в каком-либо виде, чтобы предоставить его. Это лишь образ опыта, который имею я. Могу с уверенностью сказать, что частота нахождения серьёзных багов не 1 из 100 ревью и даже не 1 из 10. Это напрямую зависит от того, какая задача решается и кто её решает. Например в случае малоопытных коллег вообще говоря количество багов в коде довольно велико, и почти каждая серьёзная задача будет содержать что-то важное, что было упущено, что не получится (скорее всего) обнаружить беглым просмотром кода. 2) Коллега работающий в той же группе не имеет чёткого понимания каждой подзадачи, которая сейчас решается даже в рамках работы над чем-то большим и связанным (эпик к примеру) 3) Возможно мы понимаем разные вещи под "задачей". Для меня это какой-то тикет, над которым человек работает в данный момент. Простой пример. Кто-то создал задачу и сказал "вот это работает неправильно". Человек пошёл фиксить проблему и в рамках неё оказалось что нужно переделать логику работы чего-то. Рассчитывать что каждый ревьювер будет знать эту часть проекта так, чтобы за "10 минут" посмотрев изменения сходу понять, что изменилось, довольно странно. Тут как минимум нужно понять как оно работало до этого, почему это не правильно, как работает новый подход и почему правильный именно он (причём желательно чтобы он был как можно ближе к идеалу). Это всё невозможно сделать в рамках беглого ревью. Гипотезы и обсуждения возможны, но вообще говоря автор вполне может и сам что-то придумать. Он же не должен обсуджать каждое своё решение до реализации? 4) Ревьювер погружается в задачу с помощью автора. Это происходит намного быстрее из-за того, что проблема уже выявлена и структурирована, решение придумано и написано. Нужно лишь понять пункты, описанные выше. Однако это так или иначе требует времени, созвоны по поводу задачи, ввод в курс дела, объяснение решения и так далее. Подводя итог я бы сказал, что описанный в видео подход к код ревью предполагает в общем случае именно ревью кода, а не логики. Потому что как я уже писал в общем случае для того чтобы понять изменения в логике, нужно погружаться в предмет обсуждения. Рабочий ли это подход? Это напрямую зависит от проекта и задач. Стоит понимать что это не единственно правильный подход

@S0ERDEVS

2 жыл бұрын

@TramFromGames смотрите, в первом комментарии вы выдвинули тезис "ревью нужно чтобы улучшать кодовую базу", в последнем вы пишете что не занимались исследованием этого вопроса и не структурировали опыт, а так же что все зависит от того какая задача решается, и кто ее решает. Получается, что вам "кажется", что кодовая база улучшается? А по факту может происходить что угодно. Я же говорю о том, что на ревью должна решаться вполне конкретная задача - как минимум один или два члена команды должны быть способны прочитать и понять код автора. Если могут, то это и есть тот результат который нам нужен. Так как кроме коллег этот код никто читать и сопровождать скорее всего не будет. И это вполне конкретная метрика, которую легко измерить, и которая легко достижима. Для меня "хороший код" - это "понятный код". И такой код получается, если автор изначально понимает, что его коллеги будут смотреть код, и он сразу закладывает это в свой рабочий процесс. А если не закладывает, то никакие заплатки и детальные ревью не помогут. Что в вашем понимании "хороший код" и какие "метрики" это показывают я вообще не могу понять, так как у вас мысль сводится к тому что все зависит от многих неконтролируемых факторов. Если сформулировать какой код вы хотите получить нельзя, то и получите вы что угодно. А формулировка "Хороший код", "хорошая кодовая база" - это способ списать часы, потраченные на бессмысленную деятельность. Я так не умею работать, я всегда стремлюсь к измеряемому и прогнозируемому результату.

да, вот столкнулся впервые, коллеги поляки на проекте просто продавливают свое мнение в ревю

что за колдунство после 18ой минуты?

Извините, а что делает жопа с глазками? Я про for (;;)

Зачем давать советы в ревью, если их можно игнорировать? Решение аппрувнуть мр/пр в любом случае принимает ревьювер, а не автор кода

@S0ERDEVS

2 жыл бұрын

Потому что климат в коллективе важнее ревью. Если ваш коллега не видит ценности в вашем комментарии - это не должно давать вам право блокировать его работу. Думаю, что ревью качественно не решит проблему найма. И нанимать джунов, заставляя на ревью опытного разраба фиксить их ошибки - это неправильно.

@timurbabkin8113

2 жыл бұрын

@@S0ERDEVS Поэтому нужно быть открытым к обсуждениям. Это нормально, если коллега не согласен с замечаниями. И не давать пройти коду в ветку "просто потому что так надо" - неправильно, это действительно не должно являться причиной для исправления, обе стороны должны аргументировать свою позицию и прийти к консенсусу, ведь ревьювер тоже может быть не прав. Возможно мы говорим об одном и том же, просто разное подразумеваем под понятием "совет".

На 20 минуте, услышав про макросы, выключил видео

Какое то противоречие у вас в видео. В начале вы говорите, что если человек будет знать, что его код будут читать по диагонали то это сильно ухудшит качество того как человек будет писать. Через пару минут говорите, что ревью должно быть быстрым и легким как выйти перекурить....

Жалкие потуги оправдать напрасную трату времени в команде. Что ж у вас за разработчики такие, что без эффекта наблюдателя они не в состоянии создавать чистый код. Работаю в Airbus-е пятый год, ничего подобного и близко нет, но при этом качество кода, на порядок выше чем в любых других проектах, включая опенсорсные. Особенно порадовало выражение “улучшение кодовой базы”. Здесь как бы подразумевается что ваша команда настолько плоха априори, и ее код требует постоянного улучшения из коробки. Мое мнение очень простое. Код ревью обычно вводят команды у которых один или несколько ресурсов из триады неограничен (время, люди, деньги). Если работает профессиональная команда, то как правило, такие практики не нужны и даже вредны. Если вам требуется код ревью в команде, значить у вас в проекте что то пошло не так. Постарайтесь это локализовать и исправить, вместо того чтобы заниматься такой ерундой. Я участвовал в нескольких проектах с кодом ревью, и качество такого “ревью” и особенно самого ревьювера было просто слезы. Возможно в неких очень специализированных проектах код ревью и можно использовать для расшаривание неких практик или компетенции, но как правило в коммерческих проектах, такой необходимости нет. У автора же наблюдается явный избыток времени, что как правило является упущением менеджмента компании. И не стоит чрезмерно увлекаться творчеством Роберта Мартина, возводя его в абсолют.

Как будь-то я говорил.