Comments 172
Можно минимизировать негативный эффект, если преред созданием пулреквеста прибраться в коммитах. Когда каждая простая мысль выносится в отдельный коммит, то перед ревью можно коммиты про одно и тоже объединить в один (ребейз + перенос коммита + сквош). Это часть саморевью.
Как и в любом творческом процессе надо найти компромисс
Разбиение по коммитам это один из способов донести до ревьювера мысль. Рассказать по шагам. Акцентировать внимание на деталях, которые легко пропустить в общем потоке.
Иногда, мысль, которая выражена в пуллреквесте, не укладывается у меня в голове. Она большая и сложная. А большие и сложные задачи я обычно декомпозирую на маленькие и простые. Перенёс эту практику на пулреквесты и мне стало удобнее.
Со схемами можно просто проверять с нижних частей к верхним. Я лично ещё стараюсь приучить себя и коммиты ребейзить согласно слоям архитектуры, правда, за счёт части истории реализации.
А коммиты удобны ещё тем, что можно проверять исправления по ходу ревью запроса на слияние.
Использование --force в стандартном flow уже запашок. Я понимаю, если надо исправить ошибки. Но вот в повседневном использовании это явно показатель нехорошего. Учитывая рутинность можно легко допустить ошибку и убить главный реп, потом будет очень весело его восстанавливать.
Также GitLab/GitHub/BitBucket позволяют защитить любую ветку в основном репозитории от перезатирания с помощью force, даже если есть админские права, чтобы никто случайно не залил что-то не то, поэтому при грамотном администрировании не вижу проблем.
Как победить не знаю.
Вторая проблема — один коммит на 2000 строк изменений.
Может быть кто-то подскажет как с этим бороться?
Как победить не знаю.А попробуйте сами рассмотреть несколько коммитов и попробовать составить к ним внятное сообщение вместо "---". Возможно проблема в том, что оно будет или бессмысленное («отрефакторил файлы foo.cpp и bar.cpp», которые и так в этом коммите очевидно присутствуют) или его просто невозможно кратко сформулировать, а развернуто описывать лень.
Да и просто описывать может быть дискомфортно, программист может невербальными категориями руководствоваться в процессе работы, к примеру видит какой-то класс, выглядящий «некрасиво» и давай его рефакторить, плюс ещё парочку зацепит. А ему предлагается потом эту концепцию сконвертировать из многомерной структры, сформированной в нейронной сети мозга в человекочитаемый вид, лишнее напряжение извилин.
Вторая проблема — один коммит на 2000 строк изменений.
- Если кодогенерация — смиритесь.
- Если джун стабильно тоннокодит — просто «Decline: Too long», после нескольких объяснений.
- Если опытный разраб — поинтересоваться «почему так», поговорить с человеком. Вежливо и уважительно поговорить. Должна же быть причина для подобного поведения. Если особых причин нет — попросить изменить подход к коммитам, показать преимущества «продвинутой» методики. Поскольку работа над оформлением может казаться коллеге скучной — польстите ему. Как только коллега оформил PR «нормально» — похвалите его.
- Если «менеджер не любит, когда время тратит на писульки» — есть резон свалить из команды или смириться.
У меня уже на второй работе есть люди которые вместо сообщения в коммитах указывают ".", или "---", или "***", или ещё что-то такого же стиля. И таких коммитов может быть штук 20 в одном MR. И кратенько в MR написано что «добавлено/исправлено/изменено то-то и то-то».
Если в MR есть вся нужная инфа — можно и забить. Если информации не хватает — поговорить с человеком, поинтересоваться «почему так». Вежливо и уважительно поговорить. Должна же быть причина для подобного поведения…
Может быть кто-то подскажет как с этим бороться?
А вы уверены, что бороться нужно? Если указание "кратенько в MR написано что «добавлено/исправлено/изменено то-то и то-то»." дает исчерпывающую информацию, то зачем писать больше?
А если коммит на 2к строк — это рефакторинг, в котором все изменения по сути одинаковые, то какой смысл дробить их на кучу коммитов?
Даже если предположить, что выполнен рефакторинг на такое большое количество строк, то чисто физически ревьювер может не заметить какой-то мелочи при просмотре казалось бы однотипных изменений (и будет баг), поэтому в общем случае, я считаю, что нужно дробить. Так можно хоть по коммитам идти и смотреть кусками что было сделано.
Иногда надо вливать большой MR и надо его ревьювить, потому что иначе никак, но делать регулярно — это бред, как по мне.
поэтому в общем случае, я считаю, что нужно дробить.
Дробить на что? Ну вот давайте воспользуемся примером из статьи — рефакторинг для замены function на =>
Как предлагаете дробить? И зачем? Считаете, такое дробление действительно в каком-то кейсе может кому-то помочь?
потому что иначе никак, но делать регулярно — это бред, как по мне.
Задач, которые требуют по 2к строк, в принципе регулярно возникать не должно. Если возникают — значит, надо делать декомпозицию, а если сделать декомпозицию — это уже будет несколько ПР, которые будут меньше.
И кратенько в MR написано что «добавлено/исправлено/изменено то-то и то-то».
Вообще там может быть даже ссылка на тикет, в котором развернуто описана задача. Беда если эта приписка в MR — это единственное место, из которого можно понять, зачем вообще вся эта деятельность была затеяна.
Например:
ABC-123 - Add HTTP Basic Authentication to request
- add login/password to config file
- pass login/password to HTTP requestПричем если коммитов много на одну задачу, то в истории сразу видно, как они сгруппированы.
В моей практике внедрялось после первого предложения на скраме — т.к. легко, быстро и не слишком фанатично.
В гите нет веток :-) Когда говорят "ветка удаляется" никакие коммиты не удаляются, удаляется лишь "автоперемещаемый именованный указатель на коммит". В Гите самое оптимальное решение проблемы связи коммитов с тикетом выглядит так:
- Именуем ветки с id тикета в названии. Например, Jira умеет прямо из тикета создавать ветки с правильными именами.
- Добавляем тривиальный
commit-msgхук, обеспечивающий, чтобы вначале описания коммита было имя ветки.
Но если у вас не публичный проект, гвоздями прибитый к ГитХабу, то лучше перейти на Mercurial, где этой проблемы нет by design и где есть ветки, как сущности первого класса.
В гите нет веток :-)
Да, есть «именованная голова», что мало меняет дело — если ее удалить, то работать с коммитами становится… не слишком удобно, мягко говоря.
В gitlab, например, при вливании запросов на слияние всегда создается коммит слияния. Или даже можно делать squash коммитов, но это повлечёт потерю истории изменений.
Но и с мерж-коммитом не все так просто. Мне удобнее именовать ветку так чтобы мне было понятно, над чем я в ней работаю. В таком случае ветка может иметь имя WJ-32_refactoring и в таком виде имя ветки попадет в коммит, и скорей всего без дополнительных настроек трекер задач уже не сможет понять, что этот коммит был связан с этой таской.
Бывает ситуация когда в процессе выполнения задачи встречаешься с каким-нибудь мелким багом и просто фиксишь его в рамках задачи отдельным коммитом. Фиксить его вне задачи, может быть не удобным, так как например меняются интерфейсы и после фикса надо будет делать мерж. В итоге получается, что все коммиты относятся к одному тикету, но среди них есть, те, что еще касаются дргуих тикетов.
Но после merge это просто коммит в master, и никакой метки о том, в какой ветке он был сделан изначально, не остается. А иногда надо устроить раскопки, чтобы выяснить, какой коммит внес некую ошибку — и тут полезно знать, к какому тикету относился коммит.
- Длинные описания коммитов не всегда хорошо. Иногда достаточно «автоматический рефакторинг с ReSharper: переименованы методы, удалены ненужные using, удалены неиспользуемые ссылки».
- Длинные описания коммитов (примеры и т.д.) не всегда хорошо. Этой информации место в таск-трекере (Jira, TFS, Redmine, etc). Иногда информацию можно частично задублировать.
- Делить один пулл-реквест на коммиты — это не единственный доступный путь. Можно разделить PR на два, в первом пред-рефакторинг, во втором — логика. Может не работать на GitHub (непривычно команде, каждого убеждать нет времени\желания), но работает в локальных командах, если заранее объяснить зачем ты так делаешь.
- Сохранять единый стиль — не всегда хорошая идея. Иногда стоит изменить стиль в одном месте «для демонстрации», потом начинать его менять всюду. Если команда в курсе об изменении стиля (точнее, одного аспекта стиля) — все будет норм.
- Сообщения на русском — спорно. С одной стороны, русский вариант быстрее и проще. С другой, над английским больше думаешь (по крайней мере, первые годы). Но тут сильно зависит от культуры команды\проекта\департамента\компании.
- Тесты. Изменилась функциональность — изменения тестов должны быть в том же коммите. Это критично при стратегии «пушим сразу после коммита» — ибо красные тесты будут ломать билд, что может отвечь DevOps и спровоцировать уведомление в рабочий чат.
За твердый стул для offline-review — отдельное спасибо.
- Практика микрокоммитов выручает всегда;
- Не использовать мердж со сквошем, иначе смысл в гита пропадает;
- Ревью кода нужно делать сразу, в тот же день;
- Обязательно нужно связывать git c таск-трекером. И писать в таске как можно больше информации в коментарии. Она пригодиться и вам самим. И это является юридическим документом, заодно;
- Связывать похожие таски. Баги редко фиксят до конца;
Длинные описания коммитов — потом будет мучительно читать историю мержей.
А вот ссылки на тикеты в трекере прямо в первой строке коммитмесседжа — это маст хэв.
Опять же, если трекер умеет подтягивать ссылки из коммитмесседжей, — это очень удобно. (А в Яндексе он умеет). Есть ли такая фича в гитхабе или гитлабе — хз.
Коммитмесседжи "шоб подавился" — все эти "---" — лечатся элементарно.
- ревьювер не пропустит, как явный саботаж
- пре-коммит-триггер в гите не пропустит (если настроить, конечно)
Тексты на английском — есть риск получить руинглиш.
Но даже ревьюверы-нейтивы (например, из Гугла, если апстримить что-то в их опенсорсные репозитории) не особо лютуют по этому поводу.
Тесты в том же коммите — необязательно.
Достаточно, чтобы тесты были в той же пачке коммитов, которую автор отсылает на сервер (git push) или после которой он нажимает перезапуск сборки.
Поскольку в гите нет нормального разграничения между микрокоммитами и завершёнными коммитами, а эмулировать это с помощью кучи пуллреквестов или какими-нибудь там подветками в рамках одного пуллреквеста — никому нафиг не надо (слишком большая цена и боль), — то проще перезапускать сборку вручную. ИМХО.
Тесты в том же коммите — необязательно.
Интересный вопрос вообще. Вот вы практикуете TDD. Red-Green-Refactor. Вопрос — когда делать коммит? После каждого цикла? После Green и после Refactor?
Я и говорю: у гита не хватает ещё одного уровня.
Микрокоммиты — это продвинутое undo-redo и синхронизация каталогов на нескольких устройствах.
Пулреквесты — это готовые продукты.
А как отмечать этапы разработки "сломал — починил — заполировал — и так семь раз"?
Ну, можно в коммитмесседже писать: "сломал", "починил", "заполировал".
Имхо, так проще всего. (И билдсервер научить: что если в коммитмесседже написано "сломал", то не надо паниковать). (Но я такого на яндексовских билдсерверах не встречал, просто теоретизирую).
Длинные описания коммитов — потом будет мучительно читать историю мержей.После вашего комментария полез проверять историю самого пробленмого скрипта в проекте. VS показывает сообщения мелким шрифтом (вроде 6 или 8), так что все помещается. Плюс, часть сообщений «обрезается». Грубо говоря, в истории отображаются первые строки сообщений. Видимо, мой стиль "НомерВетки: заголовок Пропуск строки Описание" удачно наложился на инструментарий.
Тесты в том же коммите — необязательно.Насколько я понял из статьи, автор выполняет git push сразу после коммита. То есть, «по коммиту в пачке». В таком раскладе разделение изменений кода и тестов ведет к красным билдам на билд-сервере. А это сразу сообщение в общий чат, которое отвлекает лида\девОпса и т.д. Плюс, когда ты по истории просматриваешь изменения тестов — становится сложнее выцепить коммит с соответствующим изменением функциональности (если сквоша не было). Nickolaim, ваш конфиг чтобы тесты были в той же пачке коммитов, которую автор отсылает на сервер (git push) или после которой он нажимает перезапуск сборки тоже вполне разумен.
А зачем отвлекать лида от проблем в разработчицкой ветке?
Это как-то глупо настроенный билд-сервер, в режиме повышенной паранойи.
Опять же: у меня на компе не хватит ни ресурсов, ни всех поддерживаемых конфигураций железа и системы, чтобы гонять тесты локально.
Внёс изменения, перекрестился, и отправил на билд-сервер, чтобы он рассовал сборки по ферме тестирующих агентов. Естественно, что с ненулевой вероятностью там что-нибудь отвалится.
Более того, отвалиться может даже сборка — если проект кроссплатформенный.
Или если есть баг в зависимостях, из-за чего инкрементная сборка (у меня, разработчика) отличается от сборки с нуля.
Или если за время моей работы кто-то внёс в мастер нечто, конфликтующее с моими правками. И чем раньше я это обнаружу, сделаю ребейз на вершину мастера (или ещё каким-то способом синхронизируюсь), тем лучше.
И вот всё это должно валиться в общую рассылку? Нууу не знааю.
Это как-то глупо настроенный билд-серверВот сейчас обидно было.
У нас билды падают раз в месяц, так что можно и в общий чат поспамить. Плюс, для DevOps все просто: все сообщения в один канал, никаких дополнительных действий. Видимо, слишком разные у нас проекты (по размеру\требованиям).
Или если за время моей работы кто-то внёс в мастер нечто, конфликтующее с моими правками.А как изменения в мастере могут конфликтовать с вашими правками в Dev-ветке? Или это изменения внесенные в мастер через PR?
Ну у всех стволовые ветки по-разному называются, тут может быть путаница.
Пусть главная ветка разработки — это dev. (или master, или trunk, или ещё что).
Пусть ваша ветка — это TICKET-123
build 2
dev -----------------------------> (*)
| |
| |
(*) <- TICKET-456 |
| | | build 1
+-------- | ---------------------- | ----> (*)
| | | |
| | TICKET-123 --+--------+
+---------+ |
| |
+-------------------------+
|
...Пока вы делали свой ПР, прибежал ваш коллега и успел влить (с ревью, сборками и тестами — всё по-честному) другой ПР.
На билдсервере слияние тогдашнего ствола с вашей веткой — build1 — проходило успешно, а свежего — build2 — сломалось.
Или, для пущей непонятности:
build 2
dev -----------------------------> (*)
| |
| TICKET-123 --+
| commit 2
(*) <- TICKET-456 |
| | | build 1
+-------- | ------------- | ----> (*)
| | | |
| | | |
| | +--------+
+---------+ commit 1
| |
+-------------------------+
|
...Сломалось между двумя вашими коммитами. То ли потому что вы криворукий, то ли потому что конфликт появился.
Объединить коммиты можно командой git rebase --interactive master. Напомню, что я работаю в фичебранче FEATURE-1, а master — это название основной ветки.
Я обычно делаю сквош не задумываясь через интерактивный rebase ветки на саму себя. Есть ли принципиальная разница?
Ребейз на себя не пробовал, спасибо за рекомендацию.
На самом деле оно только проблем добавляет. Ведь ты взял изменения из мастера и на них сверху свои накладываешь. Сложно понять наверное где это мерж с уже существующим в мастере кодом, а где сама логика задачи. Поэтому я лично тоже за 2 этапа: сначала логика и тесты, а потом мерж с автоматическими тестами и автоматический пуш в мастер всех изменений. Но это конечно ci-way.
Мы, что бы избежать ревью 100500 строк кода, бьем задачи на меньшие (в принципе я думаю, мы не единственные так делаем :) ) и тогда ревью получается веселее, да и попутно много решаются какие-то моменты. От постановки задачи, ее оценки и т.п.
Меня больше всего угнетает, что не все понимают необходимость ревью и относятся к этому халатно. Людей конечно не переделать, но какие можно найти слова, что бы коллеги поняли, что это не пустая трата времени и внимания ревьюера? В принципе, вопрос наверное философский :)
Поскольку ты тратишь свое время на изучение кода которое, возможно, в следующем коммите уже отсутствует. Либо я что-то не так понял.
«Приборка» кода с rebase перед пушем недаром же выполняется. Как раз с целью оставить только значимые изменения.
А от меня риторический вопрос, как отучить разрабов пушить закомментированный код?
А от меня риторический вопрос, как отучить разрабов пушить закомментированный код?
Жать decline.
А от меня риторический вопрос, как отучить разрабов пушить закомментированный код?
Ну тыкните на ревью в него и скажите удалить, делов-то.
А от меня риторический вопрос, как отучить разрабов пушить закомментированный код?
Для JavaScript видел такой экзотический способ. Добавляем правило ESlint, которое разрешает писать комментарии только с большой буквы. При комментировании кода в большинстве случаев первая буква будет маленькой. Таким образом, если закомментировать код то не пройдёт линтинг. В связке с husky это не даст закоммитить.
На практике такой способ не использовал. Это правило ESlint мне кажется необоснованным и слишком строгим.
Риторический ответ: бывает очень приятно обнаружить закоментированный код реализации твоей задачи сделанной разработчиком предыдущей, но не доведённой до прода. Остаётся лишь раскоментировать, поправить баги да написать тесты.
Код должен быть читабельным, а для этого там не должно быть закомментированных кусков.
Категоричность суждений касательно субъективных понятий (а пресловутая "читабельность" — именно такое) свойственна либо новичкам, либо идиотам. Если вы не новичок, то у меня для вас плохие новости. Я вам привёл пример, когда закомментированный код оказался очень полезным, что не согласуется с вашей картиной мира. У вас есть выбор:
- Попытаться убедить меня, что случилось что-то ужасное. Что бесполезно.
- Поправить свою картину мира так, чтобы она согласовалась с поступившими новыми фактами..
- [Сарказм]
Я вам привёл пример, когда закомментированный код оказался очень полезным, что не согласуется с вашей картиной мира.
Вы хотите сказать, что наличие примера, который демонстрирует, что что-то может оказаться полезным — достаточный повод это что-то не запрещать?
А теперь представьте, что я в 16 файлах закомментировал в разных местах строки, решающие Вашу задачу. Вам нужно:
* раскомментировать только правильные строки (а ведь в таком случае я не один там закомментированный код оставлял);
* понять, почему они были закомментированы (может там баг есть архитектурный, который не скоро, но проявится), а человек мог уже давно уволиться;
* заставить всё это в итоге работать.
Часто бывает легче, наоборот, переписать полностью чужой код, чем попытаться его использовать.
Вас переубедить у меня нет цели, но комментарии другие люди тоже читают, поэтому я всё-таки распишу, что и почему.
Спасибо, что рассказали, через какие костыли можно решить создаваемые вашей идеологией проблемы.
С соломенным чучелом вы можете упражняться сколько хотите, только пожалуйста, не у всех на виду.
Напоследок оставлю вам пример полезного кода в комментариях:
// Выключили, ибо замедляет загрузку страницы, когда данных много и пользователь ранее домотал до конца.
// Можно будет вернуть, когда перейдём с ленивого рендера на виртуальный скролл.
// Подумать о хранении в истории, а не сессии. Возможно нужно какое-то комбинированное хранилище.
// scroll_top( next? : number ) {
// return $mol_state_session.value( `${ this }.scroll_top()` , next ) || 0
// }
// scroll_left( next? : number ) {
// return $mol_state_session.value( `${ this }.scroll_left()` , next ) || 0
// }Вам сюда:
habr.com/post/145592
3 issue с зависимостями, один из которых proposal. В комментарии должна быть ссылка на соответствующий issue
Как раз в комментариях никаких ссылок на issues быть не должно, комментарий должен сам по себе содержать исчерпывающую информацию, чтобы программисту не требовалось никуда лезть для понимая, чего тут происходит и зачем нужен этот коммент в принципе.
Но так или иначе сам коммит будет содержать номер issue. Тот же `git blame` покажет всю необходимую информацию.
А насчёт исчерпывающих комментариев — спорно. Если внутри функции, то там могут быть краткие пояснения, но если код интуитивно непонятен. Существует практики самодокументирования кода и декомпозиции. Если потребовалось написать большой комментарий внутри функции, — значит что-то требуется разнести по более мелким функциям.
А вот большие комментарии к самим функциям — это хорошая практика. И тут лучше использовать системы вроде Doxygen. Впрочем, зависит от языка. В Питоне — своя система, в Яве — своя.
Существует практики самодокументирования кода и декомпозиции. Если потребовалось написать большой комментарий внутри функции, — значит что-то требуется разнести по более мелким функциям.
Это понятно. Речь о том, что если комментарий все-таки потребовался — он не должен требовать куда-то там идти и разбираться, что к чему, и о чем.
1. Предложение по рефакторингу кода, которые может понадобиться. Как люди вообще узнают, что после рефакторинга потребуется изменить кусок кода рядом с комментарием?
Подумать о хранении в истории, а не сессии. Возможно нужно какое-то комбинированное хранилище.
Это отдельный issue, причём proposal. Комментарий может лежать годами и никто не узнает, что там что-то может понадобиться переделать.
2. Когда будет реализован виртуальный скрол, люди должны будут догадаться изменить тут кусок кода о существовании которого они могут даже не знать.
Можно будет вернуть, когда перейдём с ленивого рендера на виртуальный скролл.
Мало того, значит, уже есть issue на виртуальный скролл. Два isuue можно просто связать.
3. По хорошему это уже закрытый issue, в котором можно описать всю ситуацию целиком, чтобы другие программисты могли легко разобраться в проблеме и её причинах в будущем.
Выключили, ибо замедляет загрузку страницы, когда данных много и пользователь ранее домотал до конца.
Этот issue таже можно связать с остальными. В результате остаётся только это в коде:
Выключили, ибо замедляет загрузку страницы, когда данных много и пользователь ранее домотал до конца.
Раз уж вы дое… хали до конкретного кода, давайте разберём, что тут происходит.
Это отдельный issue, причём proposal.
Пропозал по улучшению отсутствующей функциональности? Часто вы такой ерундой маетесь? Программист по быстрому накидал хинт тому, кто будет думать над этой фичей в будущем. Скажите ему лучше спасибо, что вообще что-то написал. А ведь мог забить, испугавшись вашей заградительной бюрократии.
Комментарий может лежать годами и никто не узнает, что там что-то может понадобиться переделать.
Пока не понадобится передать, этот комментарий и не нужен никому. Лежит себе, кушать не просит. Аллергики могут легко его свернуть, а то и включить автосворачивание.
Когда будет реализован виртуальный скрол, люди должны будут догадаться изменить тут кусок кода о существовании которого они могут даже не знать.
Не должны. Вот потребуется эта фича — будет задача на её имплементацию. Программист полезет в код и увидит, что всё уже написано, осталось лишь проверить то и подумать над этим.
Мало того, значит, уже есть issue на виртуальный скролл.
Нет такой issue. Есть мечта у программиста: "когда-нибудь у меня появится свободное время и я смогу сделать такую интересную штуку, от которой всем станет хорошо".
это уже закрытый issue, в котором можно описать всю ситуацию целиком, чтобы другие программисты могли легко
… не найти эту issue в десятке тысяч других. Вы вот когда-нибудь пробовали найти issue не зная её названия и формулировок описания, и даже не зная была ли она вообще? И часто вы так ищите то, не зная что?
Вы вот когда-нибудь пробовали найти issue не зная её названия и формулировок описания, и даже не зная была ли она вообще?
Да, я писал багрепорты в опенсорс. Там основное правило — проверить, не писал ли кто-либо уже такую же ошибку. Один раз попал на дубликат, но там уже была конкретная причина, до которой я сам не дошёл.
- Предложение по рефакторингу кода
Предложениям по рефакторингу в принципе нечего делать в коде, да.
Много. И разных. Например:
- дисциплина(/самодисциплина) — самая главная, тут уже в комментариях упоминали;
- читабельность — всё и везде должно быть одинакового стиля и все должны придерживаться одинаковых правил написания (сокращает время вникания в чужой код, разработка быстрее);
- беклог проблем, которые есть в проекте и которые не решены;
- отчётность — клиент может пожаловаться на баг, тогда можно легко найти этот баг и узнать, например, в какой версии он был исправлен и был ли исправлен вообще.
Я все проблемы пережил на своём опыте. И только когда своей головой дошёл до того, как правильно делать, начал изучать, что у других компаний. И оказалось, что мои позиции полностью совпадают с принятыми практиками. Только теперь я понимаю, почему у них такие практики приняты.
Если бы вам интересно было решать проблемы, а не создавать их, то вы бы с лёгкостью назвали проблемы, которые пытаетесь решить упомянутыми приседаниями, а не строить из себя обиженку. Вы, как не новичок, наверняка ведь знаете такой метод принятия решений:
- Выписать решаемые проблемы.
- Выписать все необходимые условия решения каждой из них.
- Если предлагаемое решение не входит в необходимые условия, значит это не решение.
- Выписать все создаваемые всеми необходимыми условиями проблемы.
- Сравнить создаваемые проблемы с решаемой.
Но, на будущее, просьба, не переходить на личности, не провоцировать и аргументировать. Из спора на эмоциях ни одна из двух сторон не получит никакой пользы. А мы всё-таки здесь для взаимной выгоды, а не зря тратить время. Агрессия вызывает лишь упрямство. Правило хорошей дискуссии — если почувствовали норадреналин, либо требуется сделать паузу и подумать, либо вообще прекращать дискуссию.
Такой код можно переписать компактнее, используя стрелочные функции:
const sum = arr => arr.reduce((res, i) => res + i);
В чем смысл такой сокращенной записи, тем более через константу?
Стрелочные функции следует использовать только там где это оправдано: в передачи функции в качестве параметра или когда необходимо сохранить контекст.
Ваш вариант невыразителен и не несет в себе никакой ценности, кроме использования модного синтаксиса.
IMO, вот так будет правильнее:
export function sum(arr) {
return arr.reduce((res, i) => res + i, 0);
}
В конце обязательно благодарю ревьювера. Он потратил своё время на то, чтобы вникнуть в мой код и сделать его лучше. Разве это не заслуживает приятных слов?
Попахивает излишеством. Разве это не его обязанность — ревьюить код? Назначили ревьюером — пыхти и ревьюй. За это люди, собственно, и получают зарплату.
На новом месте такого правила нет и код на ревью отправляется с целью убедиться, что сам где-то не накосячил в логике, чтобы еще кто-то взглянул свежим взглядом на твое решение, возможно что-то стоит обощить и вынести в вспомогательную библиотеку.
В описании коммитов стараюсь избавляться от всех неоднозначностей. При помощи ASCII-графики описываю тестовый пример.
Мне кажется, что это полезнее делать в виде теста.
Мне другое интересно: как заставить разработчика тратить чуть больше времени на описание коммитов, нежели на формальное «Update blablabla». По моему опыту сопротивление этой практике со стороны команды очень сильное.
По моему опыту сопротивление этой практике со стороны команды очень сильное.
Это значит, что коммиты маленькие, а с-но то описывать там и нечего.
Это значит, что коммиты маленькие...
К сожалению, ничего это не значит, кроме отсутствия дисциплины в команде.
К тому же потом всегда можно легко по истории найти коммит и сделать ему revert, в случае необходимости.
Имеет смысл писать длинное сообщение даже тогда, когда оно больше, чем само изменение. Это необходимо, чтобы все члены команды знали, что и зачем ты сделал.
Они и без сообщения откроют код и узнают.
Чем набирать git show на каждый маленький коммит вместо понятного сообщения? Ну-ну.
Я тут быстренько накидал пример качественно написанного кода на bash с исправлением ошибки в виде diff и, например, сделал коммит "Исправлена ошибка":
--- 1 2018-09-15 23:54:44.000000000 +0300
+++ 2 2018-09-15 23:54:09.000000000 +0300
@@ -1 +1 @@
-mapfile -t source_files <<(find "$source_dir" -name *.c)
+mapfile -t source_files < <(find "$source_dir" -name *.c)Во сколько раз больше времени Вы потратите на понимание этого исправления (правильно ли исправлено)? И насколько быстро Вы сможете найти этот коммит через неделю, когда Ваш коллега скажет Вам, что проблема с предварительной обработкой файлов исправлена им пару недель назад, чтобы убедиться, не та ли это регрессия, которая вдруг возникла? Ведь коммит мог быть и таким:
--- 2 2018-09-15 23:54:09.000000000 +0300
+++ 1 2018-09-15 23:54:44.000000000 +0300
@@ -1 +1 @@
-mapfile -t source_files < <(find "$source_dir" -name *.c)
+mapfile -t source_files <<(find "$source_dir" -name *.c)А ведь коммит мог быть "Исправлена ошибка предварительной обработки файлов с исходным кодом". И через пустую строчку даже могли бы быть расписаны симптомы бага.
и, например, сделал коммит "Исправлена ошибка":
А зачем, можно же номер бага в трекере указать было?
А, то есть, в баг трекере описывать мелкое изменение нужно, а коммит описать — сложно?
То есть, описывать баги в трекере — это использование инструмента по назначению, использовать вместо трекера костыли в виде описания коммита — использование инструмента не по назначению.
Используйте инструменты по назначению и вам не придется использовать костыли.
то и номер issue должен быть, и краткое описание изменений.
Что вообще такое "краткое описание изменений"?
"в строке х заменил код y на z"? а зачем это писать, если это само содержимое коммита?
а зачем это писать, если это само содержимое коммита?
Чтобы не приходилось смотреть дифф каждого коммита, пытаясь понять что и как он изменяет.
c6060c7 $mol_view_tree: don't force default value
9a3de41 $mol_app_tree - online view.tree compiler
6b3a28b $mol_textarea: tab for indentation, scroll styledЧтобы не приходилось смотреть дифф каждого коммита, пытаясь понять что и как он изменяет.
Но ведь смотреть все равно надо будет, иначе как ревью делать?
На самом-то деле все эти описания делают только хуже, т.к. вызывают некоторое чувство уверенности, что в коммите "tab for indentation, scroll styled" будет только "tab for indentation, scroll styled", что, конечно, не так.
Системы контроля версий не только и не столько для ревью используются.
Системы контроля версий не только и не столько для ревью используются.
Конечно, еще для поиска, например. И поиск сильно упрощается если коммиты достаточно крупные (чем больше коммитов, тем дольше их перебирать, с-но сложнее поиск) и с минимальным количеством информации в описании (описание же надо прочесть, чем оно длиннее — тем больше времени тратится)
Выбирая между вариантом с многочасовым ползанием среди мусора из сотен коммитов, сделанных за последние три дня, с пространным описанием в каждом, и вариантом с несколькими десятками хороших, няшных коммитов с номерами тасок — я, конечно, выберу второй вариант, т.к. в этом случае нужный код я могу найти за время порядка секунд, а не порядка часов.
Причём заметьте, что люди, которые увидят, что в коммите убрали устаревшую функцию, по крайней мере могут узнать о существовании этой проблемы. А сухой комментарий будет просто проигнорирован. Его могут даже не заметить.
Но, разумеется, если исправляется количество пробелов в отступе согласно внутренним стандартом — можно написать «стилистические правки». Если код слегка порефакторился для упрощения его понимания, — «повышение читабельности».
Изменение делается не просто так. Оно имеет определённую цель, вот эта цель и описывается.
Ну так вопрос — зачем описывать это все в коммите, если можно описать в таск-трекере, где это гораздо удобнее, а в коммите оставить ссылку?
Краткое, но внятное сообщение в коммите гораздо удобнее в этом плане. Особенно если все, что надо — это посмотреть, кто и нафига внес некий код. А для тех, кто жаждет подробностей, надо и ссылку на issue трекер указать, само собой.
Думаю, затем, что открывать ссылку — лишнее действие.
Погодите, трекер и так открыт, ведь вы в первую очередь в нем ищите.
Краткое, но внятное сообщение в коммите гораздо удобнее в этом плане.
Только не забывайте, что вам придется прочитать сотни таких сообщений, прежде чем вы найдете нужное. Сколько на это времени уйдет?
Погодите, трекер и так открыт, ведь вы в первую очередь в нем ищите.
В трекере открыта своя задача. Я же имел в виду ситуацию, когда нужно разобраться с уже существующим куском кода (git blame/svn annotate/etc).
В этом случае находится коммит, который последним трогал интересующие нас строки. Если в коммит-логе уже есть внятное описание изменений — все станет понятно уже на этом этапе. В случае же ссылки на трекер придется таки эту ссылку открывать отдельно и искать информацию уже там.
Только не забывайте, что вам придется прочитать сотни таких сообщений, прежде чем вы найдете нужное. Сколько на это времени уйдет?
Какие сотни, апчомвы? Commit id, трогавший интересующие нас строки, известен (см. выше). Остальные мессаджи можно не читать (git log/svn log прекрасно работают с указанием конкретных ревизий).
Какие сотни, апчомвы? Commit id, трогавший
интересующие нас строки, известен (см. выше).
Мы, видимо, о разных кейзах. Я говорю про поиск коммита для известной задачи, а вы зачем-то ищите задачу под известное изменение кода. Зачем и когда нужно первое — мне ясно. А вот второе? В каких случаях и для чего может понадобиться искать коммит для кода перед глазами? Я, если честно, даже выдумать такую ситуацию не могу.
В каких случаях и для чего может понадобиться искать коммит для кода перед глазами? Я, если честно, даже выдумать такую ситуацию не могу.
Счастливый вы человек. Вот без подколок — искренне завидую.
А ситуацию даже выдумывать не надо, она у меня каждый рабочий день перед глазами. Огромный проект, куча клиентов со своими особыми требованиями, несколько раз менялись требования, иногда выплывали баги, которые надо было фиксить вотпрямщас. Есть исходники по мегабайту размером (и это не шутка и не автогенерация, это код ручной выделки). Очень многие вещи можно сделать по-разному — и в одних условиях это будет правильно, в других станет ошибкой.
В общем, часто по коду непонятно, почему он написан именно так и при каких условиях он должен работать. Помогает аннотация кода: если код — древний, то в нем, скорее всего, нет ошибки (но и лезть туда не стоит — грабли могут выскочить в самых неожиданных местах). Если код трогали недавно, то смотрим внимательнее, кто и зачем это делал.
Понимаю, что наш проект — далеко не эталон с точки зрения методов разработки, но тут уж какой есть.
Если вы встречаете сильное сопротивление хорошей полезной практике, задумайтесь, возможно она не такая уж и полезная для данной команды. Разное видение, разные ценности, разная мотивация. Подумайте над причиной сопротивления, а не над тем, как всех прогнуть под своё мнение.
Мелкие коммиты сами по себе хороши по крайней мере в двух случаях: 1) Когда приходится пользоваться annotate. Непонятно, зачем нужен определённый сниппет — аннотируешь, смотришь оригинальный коммит — и там либо вся логика понятна, либо есть ссылка на issue, где описана проблема, которую он решает. Ну, или не решает (если это был тупо рефакторный коммит форматирования/переименования), и тогда сниппет можно смело менять. 2) Когда в разработке есть сравнительно долгоживущие ветки. Тогда чем меньше и логичнее отдельные коммиты — тем легче другим мержить их себе в ветки, если при этом возникают конфликты. Особенно, когда эти конфликты не текстовые, а логические (т.е. когда в vcs всё смержилось без сучков и запинок, а проект при этом "не взлетает", потому что, например, логика/архитектура в небольших деталях поменялась.
Но вот ребейзить ветку поверх свежего мастера — это многогранное решение. В смысле, нельзя сказать, что однозначно хорошо или однозначно плохо.
Про плюсы уже сказали. А минус — никто скорее всего не будет после ребейза тестировать КАЖДЫЙ из коммитов в потоке; посмотрят только, чтобы HEAD собирался/работал ок. Вроде как это, с одной стороны, никому не нужно. Но как только наступит момент поиска баги бисектом — тут-то и вскроется, что некоторые коммиты, оказывается, вообще не собираются...
В мастер надо делать мержкоммиты, по одному на пуллреквест.
Тогда бисект покажет, на каком именно пуллреквесте что-то сломалось.
А вот в рамках пуллреквеста искать проблему — это, конечно, будет весело.
Но если проблема была создана и проворонена именно в пуллреквесте, тогда бисект поможет.
А если проблема явилась следствием слияния, то тот факт, что все коммиты в исходной ветке собираются, ни о чём не скажет.
Я часами пытался разобраться в коде, по сотне раз скроллил от функции к тесту и обратно.
От того, что теперь вы разбиваете ПР на сотню коммитов кода меньше не стало. Более того, читать кода скорее всего стало больше, так для работоспособности комита приходится вносить изменения, которые в последующих коммитах будут изменены.
Писал десятки бесполезных комментариев о пропущенной точке с запятой. Всё это жутко меня раздражало.
А вы уверены, что ваша фиксация на бессмысленной пунктуации стоит затраченного времени на переписку и исправление? Попробуйте научиться при чтении игнорировать незначащую пунктуацию — жить станет куда легче.
Я избавляю ревьювера от необходимости писать десятки бесполезных комментариев о пропущенном пробеле. Всё внимание будет сконцентрировано на действительно важных вещах.
Если вы сами признаёте, что это не важные проблемы, то зачем о них вообще писать? Ну нет пробела и фиг с ним, кто-нибудь потом добавит.
При помощи ASCII-графики описываю тестовый пример. Если решению предшествовало обсуждение, где мы рисовали схемы на доске или в блокноте, то прикладываю к описанию ссылку на фотографию.
То есть вы пишете там документацию по реализованной вами функциональности. Как её искать потом? Через git blame? Или дублируете в readme.md? Тогда что она забыла в описании изменений? Вот через год встречу я в каком-то файле _.map и задамся резонным вопросом "а чем [].map не угодил?" — предлагаете ковырять всю историю за год?
Рецепт полезного код-ревью от разработчика из Яндекса
Как вот этот вот бред про документацию в описании коммита прошёл у вас ревью? :-)
Мы будем делать это тремя командами. Я выполняю их столько раз, сколько файлов изменил.
Вот вам время-то девать некуда. Может освоите наконец работу со "своим любимым WebStorm"? Он вам и изменения покажет и синтаксис подсветит и частично застейджить файл позволит и всё это в пару кликов, без изнасилования клавиатуры.
этот линтер работает "из коробки". Я вижу и исправляю проблему сразу, как только написал код, а не оставляю эту работу на потом.
Настройте уже автофоматирование. Даже линтеры умеют автоматически чинить найденные "проблемы". В кавычках потому, что большая часть их правил бесполезна, а то и вредна.
Но проект начинали тогда, когда ещё не было стрелочных функций. Если написать компактное решение, то код перестанет быть единообразным, в нём станет сложно разбираться.
От того, что где-то будет использована более лаконичная конктрукция, чем в остальном коде, сложность её понимания не возрастёт. Собственно, потому она тут и была использована, чтобы упростить понимание.
Настало время объединить коммиты пулреквеста в один
Зачем? Мерж-коммит так и так будет один.
завтра фигурные скобки после if в однострочном блоке не поставили
А зачем они там?
там переменные назвали a и b
Это вы тоже считаете не существенным? Я вот не считаю, если это не вычисление дискриминанта.
Через некоторое время в проекте творится бардак, читать код неприятно и тяжело.
Так настройте автоформатирование перед коммитом.
Они обязательны, если язык это требует.
Это что за язык такой требующий фигурные скобки в однострочниках?
О чем и пишется в ревью. Пусть настроит автоформатирование перед коммитом.
Вы не поняли, настройте это системно. Чтобы при разворачивании рабочего окружения автоматически устанавливались хуки и настройки редактора.
husky, editorconfig, tsconfig, directory based idea config — вот это вот всё.
завтра фигурные скобки после if в однострочном блоке не поставили
Вы так говорите, словно это что-то плохое…
Через некоторое время в проекте творится бардак, читать код неприятно и тяжело.
Для этого есть инструменты автоформатирования же. Та же Idea умеет, при чем может дае перед коммитом переформатировать автоматически.
Вы так говорите, словно [не поставить фигурные скобки после if в однострочном блоке] это что-то плохое…
Да, это ужасно. Если язык позволяет поставить фигурные скобки после if их надо ставить всегда. Если не поставить, то кто-нибудь обязательно напишет туда ещё одну строку с соответствующим выравниваем и будет думать, то она выполняется только когда срабатывает if.
Кроме того, если надо залогать что-то для отладки, когда срабатывает if а потом убрать логи — то радости добавлять скобки, а потом удалять — немного.
Да, это ужасно. Если язык позволяет поставить фигурные скобки после if их надо ставить всегда. Если не поставить, то кто-нибудь обязательно напишет туда ещё одну строку с соответствующим выравниваем и будет думать, то она выполняется только когда срабатывает if.
Секундочку! Если кто-то видит, что в if нет фигурных скобок и что-то туда дописывает с ожиданием того, что работать будет, как блок, то это не проблема однострочного if, это проблема компетентности того, кто вставляет строку. Во-вторых категоричное «всегда» опровергается практикой. К примеру такой код:
public void doSomething(String param) {
if (param == null) throw new RuntimeException("...");
if (param == "") throw new RuntimeException("...");
if (param.equalsIgnoreCase("error")) throw new RuntimeException("...");
doSomething(param, DateTime.now());
}переписанный с расстановкой фигурных скобок неоправданно разрастется, и читаться будет намного хуже — проверено практикой.
Кроме того, если надо залогать что-то для отладки, когда срабатывает if а потом убрать логи — то радости добавлять скобки, а потом удалять — немного.
Если вам не хватает дебаггера, и вы вставляете логгер для отладки, то имеет смысл оставить этот логгер в этом месте навечно, просто настроить его на уровень debug. В остальном следует задаться вопросом, что происходит в коде, если у вас возникают подобные желания там, где работает однострочник. Может быть логгировать стоит не рядом с ним, а внутри него?
Секундочку! Если кто-то видит, что в if нет фигурных скобок и что-то туда дописывает с ожиданием того, что работать будет, как блок, то это не проблема однострочного if, это проблема компетентности того, кто вставляет строку.
Дело не в компетентности, а просто в вероятности допустить случайную ошибку. Если определенное оформление кода снижает вероятность ошибки по сравнению с другими вариантами оформления — стоит всегда предпочитать первое.
В общем-то ведь иногда можно и вовсе писать код в одну строку. А если у кого-то возникают проблемы с чтением/редактированием такого кода — ну это просто человек некомпетентен. Разве не так?
Если кто-то видит, что в if нет фигурных скобок и что-то туда дописывает с ожиданием того, что работать будет, как блок, то это не проблема однострочного if, это проблема компетентности того, кто вставляет строку.
Ну да, а если кто-то передаёт в метод строку с ожиданием того, что она будет работать, как чисто с плавающей точкой, то это проблемы не языка без строгой типизации, а того, кто пишет код. Есть такая точка зрения, да. Но по моему мнению, если можно устранить человеческий фактор, его нужно устранить.
К примеру такой код
переписанный с расстановкой фигурных скобок неоправданно разрастется, и читаться будет намного хуже — проверено практикой.
Да нет, вот так выглядит лучше и читается намного лучше. Тоже практикой проверено.
public void doSomething(String param) {
if (param == null) {
throw new RuntimeException("...");
} else if (param == "") {
throw new RuntimeException("...");
} else if (param.equalsIgnoreCase("error")) {
throw new RuntimeException("...");
}
doSomething(param, DateTime.now());
}Если вам не хватает дебаггера, и вы вставляете логгер для отладки, то имеет смысл оставить этот логгер в этом месте навечно, просто настроить его на уровень debug
Когда как. Бывает, что имеет смысл оставить его навечно, бывает, что не имеет.
Да нет, вот так выглядит лучше и читается намного лучше. Тоже практикой проверено.
Вы это серьезно? О_о Лучше читается? Лучше выглядит??? Ладно, проехали… *ушел переосмыслять жизнь*
Вы это серьезно?
Серьёзно
Лучше читается?
Да, условие на одной строке, действие на другой. Читается лучше.
Лучше выглядит???
Ну это конечно субъективно, но я подозреваю, что лучше выглядит потому, что читается лучше. А у вас критерий красоты похоже — количество строк. Хотя, конечно в чужое сознание проникнуть сложно ))
Ладно, проехали… ушел переосмыслять жизнь
Поинтересуйтесь у кого какие стандарты оформления кода. Я в своё время нашёл много таких вещей, которыми мне казались такими же дикими, как вам простановка скобок всегда и везде. Очень расширяет сознание.
if (a == b) doSomething(a);Такие конструкции разработчик обязан читать влёт. Вбахать туда что-то после точки с запятой может новичок без знания языка вообще, но это просто надо знать, это относится к синтаксису языка. Огораживать скобками такие «возможные ошибки» — просто потеря времени и сил.
Такие конструкции разработчик обязан читать влёт.
Также ему неплохо бы знать, что надо писать не a == b, а a === b )) .
Вбахать туда что-то после точки с запятой может новичок без знания языка вообще
Вбахать туда что-то после точки с запятой может кто угодно. Этот факт отражён в корпоративных стандартах компаний, которые поняли это на собственном опыте.
но это просто надо знать, это относится к синтаксису языка
Когда части синтаксиса языка являются источником ошибок, лучше ими не пользоваться. Например, не надо использовать в JS for… in…. Вообще. Не смотря на то, что иногда эта конструкция работает нормально и просто надо знать, когда она работает нормально, а когда нет.
Огораживать скобками такие «возможные ошибки» — просто потеря времени и сил.
В стандартах кодирования нередко отражено обратное.
== и === в JS имеют разный функционал, нужно знать и то, и то, и использовать по назначению
Смысл у этих операторов действительно разный, знать разницу действительно неплохо и зачастую те, кто эту разницу понимает, == не используют. Или используют в случаях, когда без этого не обойтись. Я таких случаев не припомню.
Но я рад, что по остальным пунктам у нас разногласий не возникло ))
Но я всё же настаиваю, что если язык позволяет делать так, надо делать так. Либо уже переходить на TS, когда хочется статических типов
Но я всё же настаиваю, что если язык позволяет делать так, надо делать так.
Вы, наверное хотели сказать можно, а не надо, да? Если язык позволяет использовать оператор равенства с приведением типов, то можно его использовать?
Либо уже переходить на TS, когда хочется статических типов
=== не про статическую типизацию, он про строгую. В TS точно так же есть == и === с теми же проблемами. Но любители === и любители статической типизации и вправду зачастую лучшие друзья )).
В остальном — как быть, если TS еще не сделали, а баги, связанные с == уже изрядно замучали? По-моему, вывод очевиден.
Этот факт отражён в корпоративных стандартах компаний, которые поняли это на собственном опыте.
В стандартах кодирования нередко отражено обратное.
В корпоративных стандартах можно встретить временами настолько лютый шлак, что лучше на них не ссылаться. Вообще апелляция к авторитетам штука скользкая, а «стандарты кодирования компании» на такой авторитет не тянут даже близко.
В корпоративных стандартах можно встретить временами настолько лютый шлак,
Временами, наверное можно, но в основном лютого шлака там нет. В основном там рекомендации, выработанные годами практики.
что лучше на них не ссылаться.
А на что лучше ссылаться?
Вообще апелляция к авторитетам штука скользкая,
Ну апелляция к авторитету немного лучше, чем апелляция к собственному мнению.
а «стандарты кодирования компании» на такой авторитет не тянут даже близко.
Гугл для вас не авторитет?
Гугл просто раскрученная большая и успешная компания. Мне, например, не нравится гугловский стандарт кодирования для C++. Он имеет странные ограничения и не особо «читаем». Возможно для самого гугла он подходит хорошо. Так что то, что подходит гуглу подходит именно гуглу.
Аналогично с Яндексом, Майкрософт, JetBrains, Касперский, DINS, Facebook, Whatsapp и прочими известными успешными компаниями. Они что-то используют — отлично, можно на это посмотреть, но не стоит слепо это тут же использовать у себя. Выбранный продукт может подходить конкрено для их ситуации, он может быть доработан ими для себя. В конце концов, в этих компаниях работают обычные люди которые тоже могут ошибаться. И кто-то что-то протащил по ошибке и теперь они этим пользуются, так как заменить будет дороже.
Когда я разбирался с протокольными буферами у меня упал авторитет Гугла в глазах. А реализация генератора для С++ вообще не понравилась, — раздутый результат, с большим количеством выделений памяти.
Огораживать скобками такие «возможные ошибки» — просто потеря времени и сил.
Это как раз экономия времени и сил. Никаких затрат в том, чтобы сразу поставить скобки — нет, зато ожидание пользы — ненулевое (даже если вероятность ошибки — крайне мала).
public void doSomething(String param) {
if (param == null) { throw new RuntimeException("..."); }
if (param == "") { throw new RuntimeException("..."); }
if (param.equalsIgnoreCase("error")) { throw new RuntimeException("..."); }
doSomething(param, DateTime.now());
}Пример практически не отличается от вашего. Скобки стоят.
кто-нибудь обязательно напишет туда ещё одну строку с соответствующим выравниваем и будет думать, то она выполняется только когда срабатывает if
они не решают — от этого «кого-нибудь» все равно потребуется осознать что и куда он вставляет, а для всех разумных людей такие скобки станут визуальным шумом, плюс автоформаттеры могут такой вариант превращать из одной строки в три.
для всех разумных людей такие скобки станут визуальным шумом
1. Не для всех. «Разумные люди» — категория слишком размытая.
2. Не станут. Ибо дело привычки.
Что касается «проблемы компетентности того, кто вставляет строку» после if без фигурных скобок — я вас уверяю, такие ляпы допускают все разработчики независимо от уровня компетенции. Конечно, чем человек опытнее, тем меньше вероятность, но даже супер-пупер гуру не застрахованы от таких ошибок.
Но, по моему мнению, однородные операции лучше в if else оборачивать, чтобы видно было, что это какай-то единый оператор выбора.
Т.е. мне кажется правильным
if (param == null) throw new RuntimeException("...");
if (param == "") throw new RuntimeException("...");
if (param.equalsIgnoreCase("error")) throw new RuntimeException("...");
doSomething(param, DateTime.now());
Но, так же будет правильным
if (param == 1) {
do1();
} else if (param == 2) {
do2();
} else {
doDef();
}
Из статьи вынес ещё пару полезных вещей для себя на будущее.
И ещё пункт насчёт названия коммитов: мы просто называем следующим образом — JiraTaskName: commit summary. Например, ABC-1001: Сделан вывод ошибки при вводе неправильных данных. Это достаточно удобно для дальнейшей работы, у нас GitLab, ссылки на таски в джиру проставляются автоматически.
У нас тоже используется Jira и Gitlab. Как мне кажется, Jira больше про управление проектами, а Gitliab про управление репозиториями. Соответственно, issue в Gitlab привязана к репозиторию, а в Jire к проекту. Jira более удобна для управления процессом, так как заинтересованным лицам не нужно думать в терминах репозиториев и вообще знать, что они существуют.
В отличие от RedMine, Jira это только система управления проектами, там не будет вики для проекта и репозитория для него. В качестве wiki предлагается использовать Confluence, а отображение коммитов связанных с issue достигается интеграцией с Bitbucket. Вроде, можно настроить, чтобы в коммите можно было указать время затраченное на задачу и оно учется в Jira, можно, чтобы issue при ее упоминании переходила в определенное состояние, но мы этим не пользовались.
Можно ли jira так же интегрировать с Gitlab — думаю, что да, но я этим не занимался и это только мое предположение.
А в плане задач меня интересовала следующая схема. Я создаю множество задач, каждой выставляю оценочное время выполнения. Проставляю между ними зависимости. Затем делаю что-то вроде milestone'а, указывая, какие программисты в нём должны участвовать. Система в итоге должна построить диаграмму Ганта для скорейшего достижения milestone'а. В идеале ещё было бы хорошо, чтобы каждой задаче можно было проставлять оценку сложности для корректного распределения между юниорами, мидлами и сеньорами.
Пока по описаниям не нашёл ни одной такой системы.
Такого "из коробки" Jira не умеет. Только если писать плагины для нее самому. Зависимости задач есть, оценка сложности — в Jira можно добавлять собственные поля. На моей прошлой работе было сделано такое: можно было выбрать разработчиков участвующих в проекте и распределить между ними задачи (вручную), при этом показывалась загрузка разработчика на указанный период по другим проектам, но это наш отдел сопровождения Jira ее модернизировал.
Это не то что Вам нужно, но если была сделана такая работа, то как мне кажется, можно расширить автоматическим распределением задач о разработчикам.
Я тоже подобного не встречал, но выглядит как полезная и востребованная вещь. Можно попробовать поискать в плагинах для Jira, по этой теме попалась пара плагинов: Assign & Escalate for Jira и Issue Alternative Assignee. Но это поверхностный поиск, может можно копнуть глубже и найдется, что-то более подходящее.
Пока по описаниям не нашёл ни одной такой системы.
Их и не существует, потому что оптимальную диаграмму ганта нельзя построить по оценочному времени. Только по четко заданному, которое гарантированно не изменится.
Задачи так или иначе никогда не укладываются в оценочные сроки.
И, с-но, диаграмма ганта не может быть построена. По-этому описываемых вами систем не существует и не может существовать.
Но ничего не мешает её перестраивать по ходу решения задач.
А какая от нее польза тогда? И уж тем более какая польза именно от оптимальной диаграммы? Просто та диаграмма, что будет оптимальна с четко заданным временем, уже не будет оптимальна в общем случае, если добавить дисперсию.
А какая от нее польза тогда? И уж тем более какая польза именно от оптимальной диаграммы? Просто та диаграмма, что будет оптимальна с четко заданным временем, уже не будет оптимальна в общем случае, если добавить дисперсию.
Я ещё только вникаю в Agile. Но, если я правильно понимаю терминологию, это автоматизация Scrum-мастера.
Но, если я правильно понимаю терминологию, это автоматизация Scrum-мастера.
Автоматизацией она является в том случае, если дает валидный результат.
Вы же не назовете "автоматизацией уборщика" систему из набрасываемых на вентилятор кусков говна? Нет. Почему? Потому что в итоге работы этой системы чище не становится.
В некоторых случаях сложная задача будет ставится начинающему разработчику, так как с учетом поправочных коэффициентов на сложность задачи и опыт разработчика получится, что он ее как раз сделает ко времени когда остальные разберутся с прочими проблемами. Может ли этот срок изменится — да, может. Но, мне кажется, что это недостаточная причина, чтобы отказываться от такого планирования или считать его бесполезным. Так как, в конечном счете, все зависит от первоначальных оценок трудоемкости той или иной задачи. В ряде случаев есть возможность не думать сколько времени займет выполнение той или иной задачи, а делать до тех пор пока она не будет сделана, но это не везде так и не всегда.
Описанная в комментариях автоматизация, дает вполне себе валидный результат — распределение задач между разработчиками в зависимости от сложности задачи и уровня разработчика, таким образом, чтобы с одной стороны загрузить каждого, а с другой минимизировать срок завершения.
Дает, но только при условии, что сроки однозначно заданы и не поменяются.
Если они могут поменяться — то уже не дает.
Можно с тем же результатом просто рандомно раскидать задачи.
И в общем — статья действительно хороша. Прям идеальная последовательность действий, которую наконец кто-то взял и четко сформулировал.
Автору большое спасибо!
Рекомендую прочитать книгу ProGit, все оттуда с первого раза, мне было, сложно запомнить, да и не к чему оно, но зато есть понимание куда смотреть если возникает вопрос.
> git status
> git diff comments.js
> git add comments.js
Я выполняю их столько раз, сколько файлов изменил.
А не удобнее ли git add --interactive? Я вот без него жить не могу: очень удобно и ревьювить перед коммитом и разбивать код на микрокоммиты.
Если вы пользуетесь WebStorm, то зачем писать коммит сообщение в Vim? В настройках Intellij IDEs есть настройки Commit Dialog, там можно указать длину subject line, длину body. Получается аналогично тому что в Виме. Плюс есть базовый spell checker.
А команду git log чтобы сформировать описание пулл реквеста когда и как используете? Выполняете команду, а потом вручную копируете текст и затем вставляете в GitHub?
Мне хочется все команды для работы с git вызывать в одном месте. Могу ли я в IDE выполнить ребейз с флагом
--interactive или лог с ключами --pretty='%h: %B' --first-parent --no-merges --reverse? Если можно, с удовольствием полностью перееду в IDE.А команду git log чтобы сформировать описание пулл реквеста когда и как используете? Выполняете команду, а потом вручную копируете текст и затем вставляете в GitHub?
Да, выполняю в консоли и копирую вывод в описание к пулреквесту.
Рецепт полезного код-ревью от разработчика из Яндекса