?

Log in

No account? Create an account
November 2016   01 02 03 04 05 06 07 08 09 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30
cartoon

Миф о документации, продолжение

Posted on 2011.06.18 at 23:06

Comments:


Gaperton
gaperton at 2011-06-19 10:13 (UTC) (Link)
Ревью исправления орфографии занимает одну минуту. Так что разумеется буду. А вдруг в изменениях _не только_ исправление орфографической ошибки? А вдруг там какой-то левый код зацеплен, по принципу, изложенному выше?

Эффект от такой политики на моральный климат - попрошу объяснить. :) Че-то за 6 лет в CQG я, работая программистом и тимлидом, не заметил отрицательного эффекта именно от этой политики - только положительные.
malica_dee
malica_dee at 2011-06-19 13:45 (UTC) (Link)
Ну чуть ниже по треду тобой же был изложен легальный в вашей конторе способ не делать лишнюю работу, если бы он легальным не был, то результатом было бы накапливающееся раздражение и "да ну его, когда QA тикет заведут, тогда и исправлю".

У нас предпочитают вместо "прицепа" к тикету, делать отдельный коммит, в котором честно написано
Bug ID: n/a
Desc: typo corrected
Если все капельку серьезнее, чем исправление "p" на "o" - то будет там и ревьюер. Если речь идет о чем-то важном - ну тогда пишем тикет, открываем баг, заказчик должен знать, над чем разработчик трудился.

Что характерно - это облегчает чтение каментов к коммитам, "прицепы" считаются скорее порочной практикой.
Gaperton
gaperton at 2011-06-19 14:01 (UTC) (Link)
Это ревьювится за минуту, не вижу причины в этом случае не делать ревью прямо с экрана, и не писать фамилию ревьювера.

А вот дырку в процессе, через которую можно шаловливые ручонки в код запустить - вижу.
malica_dee
malica_dee at 2011-06-19 14:12 (UTC) (Link)
Собственно зависит от уровня внутренней дисциплины в команде. Если она достаточно высокая, то никто в эту дырку и не полезет.
Gaperton
gaperton at 2011-06-19 14:19 (UTC) (Link)
"Уровень дисциплины" - это не данность, которая дана свыше. Уровень дисциплины зависит от руководителя.
malica_dee
malica_dee at 2011-06-19 14:18 (UTC) (Link)
Кстати, если Bug ID хочется непременно сделать обязательным, чтобы избежать "прицепов" можно создать отдельную всегда открытую задачу имени неземной красоты кода и граммар-чистоты и ставить ее id в такие коммиты.
Gaperton
gaperton at 2011-06-19 14:33 (UTC) (Link)
Это бессмысленно. Наведение красоты не модет являться целью, и соответственно, задачей. Нечего статистику этой ерундой портить.
malica_dee
malica_dee at 2011-06-19 20:15 (UTC) (Link)
Смысл исключительно в том, чтобы комит, который содержит косметические изменения был отдельным, и содежал комментарий, который говорит, что это косметическое изменение, коммит, который содержит исправление орфографическую ошибку, был отдельным и содержал информацию о том, что здесь была исправлена орфографическая ошибка и ничего больше. А не так, что меняли алогоритм согласно багу №хххх и исправили орфографические ошибки (в другом модуле) и все это одним комитом, и в каменте (который, как мы помним видно, из среды разработки) видно и про алгоритм, и про орфографию в обоих модулях, где были справления (а высота и ширина окна чсх ограничены).

Наведение красоты - это повышение читабельности и удобства сопровождения. Не такая уж бессмысленная вещь, ИМХО. Впрочем, если статистика дороже реального положения дел, то, конечно, не стоит этим заниматься.
Gaperton
gaperton at 2011-06-19 20:26 (UTC) (Link)
Статистика по задачам как раз должна отражать реальное положение дел. А положение реальное тогда, когда постановка задачи одинаково понятна как программисту, так и пользователю.

Поэтому - не надо заводить левых задач в трекере.

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

К примеру - знаешь, чем чреваты "косметические" на взгляд некоторых программистов переименования процедур и полей в T-SQL? У нас - из-за таких "безобидных" на первый взгляд изменений, твоих коллег могут поднять ночью телефонным звонком, чтобы они исправили проблемы в загрузке сделок до следующего утра.
malica_dee
malica_dee at 2011-06-19 21:28 (UTC) (Link)
Ну у нас и нету левых, просто есть возможность баг id поставить n/a. Чтобы не писать "прицепов".

Слушай, ну давай не путать мягкое с теплым, я не думаю, что то, что переименование процедур и полей в T-SQL может вызывать такие последствия такая уж тайна для тех, кто на этом проекте работает не вчера. Новичков всегда надо отслеживать очень плотно, но всегда есть тот порог, с которого проверки становятся просто непроизводительной тратой рабочего времени.
Gaperton
gaperton at 2011-06-19 21:34 (UTC) (Link)
> Ну у нас и нету левых, просто есть возможность баг id поставить n/a. Чтобы не писать "прицепов".

Ага. И обойти при этом code review. "Потому, что оно мешает работать".

Знаем, слышали.
malica_dee
malica_dee at 2011-06-19 21:59 (UTC) (Link)
Кстати, n/a нифига не значит, что не будет кодревью. В случае, когда изменили строку сообщения в ресурсах его может не быть, не или если закоммитили временные иконки, которые нужны пока дизигнер не сделает на их место нормальные. Что-то в этом духе. В остальных случаях кодревью таки будет - никто себе не враг.
Gaperton
gaperton at 2011-06-19 21:43 (UTC) (Link)
> Слушай, ну давай не путать мягкое с теплым, я не думаю, что то, что переименование процедур и полей в T-SQL может вызывать такие последствия такая уж тайна для тех, кто на этом проекте работает не вчера.

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

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

Простая мысль, вроде, нет? А как долго я ее объясняю :) Представь, что это происходит на работе. Ну вот как тут без штрафов и взысканий, а? :) Не доходит же по другому.
malica_dee
malica_dee at 2011-06-19 22:11 (UTC) (Link)
Я там примеры приводила, в общем. Реально есть места, где накосячить исключительно сложно, ближе к невозможно.
Можно делать ревью и в таких случаях, нивапрос. Но это немного снижает производительность.
Dmitry Antonyuk
lomeo at 2011-06-22 11:00 (UTC) (Link)
> Простая мысль, вроде, нет? А как долго я ее объясняю :)

Не брать на работу программистом тех, кто не в состоянии понятным образом излагать свою мысль на родном языке. Категорически. Эта работа им противопоказана.

:)

Изменение надо проверять, конечно, но зачем для примитивных изменений задействовать такой дорогой ресурс? Проверить можно и с помощью существующих тестов, например.
Gaperton
gaperton at 2011-06-22 20:40 (UTC) (Link)
Затем, что раз программа пишется в первую очередь для человека, то и читать ее должен в первую очередь человек.
Dmitry Antonyuk
lomeo at 2011-06-23 06:06 (UTC) (Link)
Разумно, но не следует. Твоё "читать" означает "проверять", а код пишется в первую очередь не для того, чтобы его проверял человек.

Сама по себе проверка навредить, конечно, не может. Но сам этот процесс ничего не производит, а время тратит. В каждом конкетном случае оно может и окупается, а может и нет. У меня нет данных. Рассуждать можно, но чёткого мнения у меня тоже нет, а выводов о необходимости код ревью для всех коммитов всегда-всегда я не вижу, да и сделать не могу.

Поэтому давай так - я, наверное, попробую на одном из проектов договориться о полном код ревью и посмотреть, что получится. Пока у нас ревью проходит код новичков, и код, который автор попросил (!) проверить. Увижу существенную разницу - отпишусь.
Gaperton
gaperton at 2011-06-23 21:21 (UTC) (Link)
> а код пишется в первую очередь не для того, чтобы его проверял человек.

Это не более чем вопрос твоего отношения. Схоластика.

> Сама по себе проверка навредить, конечно, не может. Но сам этот процесс ничего не производит, а время тратит.

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

> У меня нет данных.

У меня есть.

> Поэтому давай так - я, наверное, попробую на одном из проектов договориться о полном код ревью и посмотреть, что получится.

Договориться мало, надо уметь его правильно проводить. Люди далеко не сразу учатся это делать, даже когда знают, как правильно - требуется несколько месяцев.

> Увижу существенную разницу - отпишусь.

Боюсь - не увидишь. Разница от этих мер наблюдается на длинных интервалах времени - и надо еще знать, на что смотреть.
Dmitry Antonyuk
lomeo at 2011-06-24 09:16 (UTC) (Link)
Т.е. советуешь забить и пользовать уже налаженный процесс?

> и надо еще знать, на что смотреть.

На что?
Gaperton
gaperton at 2011-06-24 10:20 (UTC) (Link)
> Т.е. советуешь забить и пользовать уже налаженный процесс?

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

> На что?

Например, на то, насколько проще тебе будет перебалансировать задачи и работы по поддержке, когда вследствие кодревью у разработчиков появится перекрытие по компетенциям без каких либо дополнительных усилий с твоей стороны.
Dmitry Antonyuk
lomeo at 2011-06-24 16:25 (UTC) (Link)
Хороший аргумент, спасибо!
Gaperton
gaperton at 2011-06-22 22:11 (UTC) (Link)
И, что характерно, о том, что изменение "примитивное", мы знает только со слов человека. А люди - они такие ненадежные. :)

Поэтому надо проверять. Если изменение примитивное - то и его проверка примитивна.
Dmitry Antonyuk
lomeo at 2011-06-23 06:10 (UTC) (Link)
Тоже не следует.

Например, потому что для некоторых проектов баланс "быстрее-корректнее" смещён в сторону "быстрее" сильнее, чем в других.
Previous Entry  Next Entry