LINUX.ORG.RU

Нужны ваши мысли по code review

 ,


2

3

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

На данный момент, вся логика максимально отдалена от структур проекта, репозиториев, коммитов и всего такого, поэтому можно комментировать любой кусок кода или произвольный текст, что использовалось на практике для «тыкания носом» в выдержку из какого-нибудь лога или YAML-конфига.

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

Хотелось бы услышать ваши мысли и пожелания, в первую очередь по интерфейсу - как это должно выглядеть для максимального удобства проведения code review.

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

★★★

Например, в том же gitlab есть комментарии к коммитам… Классическое ненужнос. Git-diff с комментариями

rtxtxtrx
()

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

Гугл использует.

gag ★★★★★
()
Ответ на: комментарий от gag

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

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

alex0x08 ★★★
() автор топика
Ответ на: комментарий от alex0x08

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

alysnix ★★★
()

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

Используется, поверьте. Мне геррит не нравится тем, что их концепция change и patchset, хоть и удобна при ревью, но как бы обходит стороной непосредственную рулёжку ветками. В bitbucket ветки git один-в-один переходят из локальных веток в пулл-реквесты и смердживаются. В любой момент можно посмотреть дерево коммитов. В геррит рулёжка ветками отдельно, а ревью отдельно, и создаются «мусорные» ветки только ради реботы с change.

seiken ★★★★★
()
Ответ на: комментарий от alysnix

Я имею ввиду следующую ситуацию: есть некий проект с устоявшейся кодовой базой, достаточно большой и сложной. Есть команда разработки, которая занимается его доработкой и сопровождением. Есть риск «все сломать» если кто-то из разработчиков выложит большое количество правок сразу.

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

Чтобы все это соблюсти и используется code review, когда перед тем как изменения попадут в основную версию их проверяют и одобряют.

Соответственно сей сложный процесс я и автоматизирую, но в данный момент на самом низовом уровне.

alex0x08 ★★★
() автор топика
Ответ на: комментарий от seiken

В геррит рулёжка ветками отдельно, а ревью отдельно, и создаются «мусорные» ветки только ради реботы с change.

Ну да, я уже морально готов к тому что придется фактически git сервер затягивать в проект, для реализации цельного решения.

alex0x08 ★★★
() автор топика
Ответ на: комментарий от alysnix

ревью кода делает руководитель проекта или руководитель группы

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

static_lab ★★★★★
()
Ответ на: комментарий от static_lab

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

alysnix ★★★
()

У нас вся работа на гитхабе в основном, ибо опенсорц, поэтому я как-то привык к его интерфейсу ревью: дифф по файлам в PR с комментами и саджестами в нужных местах. На гитлабе в целом похожая картинка.

Zhbert ★★★★★
()
Ответ на: комментарий от Zhbert

Это понятно, но врядли имеет смысл повторять 1 в 1 интерфейс. Вообще, из того что заметил - нужно делать что-то вроде «авторского ревью», с черновиками и статусами.

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

alex0x08 ★★★
() автор топика
Ответ на: комментарий от static_lab

вам не нравится принцип субординации и иерархии ответственностей при разработке по? предпочитаете атаковать задачу стаей, и потом друг дружке досматривать код?

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

alysnix ★★★
()
Ответ на: комментарий от alysnix

вам не нравится принцип субординации и иерархии ответственностей при разработке по? предпочитаете атаковать задачу стаей, и потом друг дружке досматривать код?

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

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

alex0x08 ★★★
() автор топика
Ответ на: комментарий от alysnix

вам не нравится принцип субординации и иерархии ответственностей при разработке по?

Так как там в армии с разработкой?

предпочитаете атаковать задачу стаей, и потом друг дружке досматривать код?

Коллективно обсуждаем, коллективно договариваемся о правилах, коллективно их соблюдаем

у вас ничего не получится

В сильной и слаженной команде профессионалов всё получается

это плоский метод

Базарный метод и Agile/Scrum вам о чём-нибудь говорят?

команда первого уровня

«Если вы такие умные, почему строем не ходите?»

группа погромистов, готовая разбежаться кто куда

Нужно, чтобы в команде было общее понимание задач и целей

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

git blame

static_lab ★★★★★
()
Ответ на: комментарий от static_lab

В сильной и слаженной команде профессионалов…

ну вот откуда вы знаете, что вы сильные, что слаженные и что профессионалы?

alysnix ★★★
()
Ответ на: комментарий от static_lab

git blame

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

alysnix ★★★
()
Ответ на: комментарий от alysnix

ну вот откуда вы знаете, что вы сильные, что слаженные и что профессионалы?

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

static_lab ★★★★★
()
Ответ на: комментарий от dataman

Чет я видимо слишком давно на свете живу чтобы верить вот в такое:

One of the innovative features of Fossil is its built-in web interface. This web interface provides everything you need to run a software development project:

Ticketing and bug tracking
Wiki
On-line documentation
Technical notes
Forum
Chatroom
Timelines
Full text search over all of the above
Status information
Graphs of revision and branching history
File and version lists and differences
Download historical versions as ZIP archives
Historical change data
Add and remove tags on check-ins
Move check-ins between branches
Revise check-in comments
Manage user credentials and access permissions
And so forth... (some examples)

Есть какая-то практика использования?

alex0x08 ★★★
() автор топика
Ответ на: комментарий от static_lab

ну вот скажите. чтобы построить современный многоквартирный дом достаточно команды «сильных, слаженных профессионалов»?

нет. для этого надо иметь архитекторов, прорабов, бригадиров, бетонщиков, сварщиков, электриков, такелажников, крановщиков…и так далее. компетентно работающих строго каждый на своем месте, в пределах должностных инструкций и ответственности. и даже необязательно им знать друг друга, и тем более о чем-то «договариваться». а ваша «команда прекрасных специалистов» в таком деле, в 99 проц. случаев есть проcто шайка гастеров из средней азии, что в первый раз увидели лифт.

alysnix ★★★
()
Ответ на: комментарий от alysnix

А я говорю про строительных архитекторов, которые разрабатывают весь проект дома. Они не проектируют дом «на верхнем уровне», они выдают полный комплект документации, по которой любая команда строителей сделала бы идентичный дом. В программировании это именно что написание программы.

static_lab ★★★★★
()
Ответ на: комментарий от static_lab

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

alysnix ★★★
()
Последнее исправление: alysnix (всего исправлений: 1)
Ответ на: комментарий от alex0x08

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

static_lab ★★★★★
()
Ответ на: комментарий от alysnix

весь проект может помещаться в одной голове

но вы утверждаете, что проект должен ревьюить исключительно один только руководитель

static_lab ★★★★★
()
Последнее исправление: static_lab (всего исправлений: 1)
Ответ на: комментарий от static_lab

Например делали портал аналитики.

какие языки, технологии, фреймворки использовлись?

продукт уровня десктопной ос ваши несколько сеньоров смогут разработать за приемлемое время?

alysnix ★★★
()
Ответ на: комментарий от alex0x08

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

Обычно, эти несколько человек ведут беседу, что уже не просто каша.

Вообще, из того что заметил - нужно делать что-то вроде «авторского ревью», с черновиками и статусами.

Это же есть в Github & co. Там ты делаешь аудит, кидаешь кучу комментариев и выставляешь статус (request changes, approve).

Честно говоря, я так и не понял, чем тебя традиционный подход не устраивает.
Paster выглядит интересно, в том смысле, что можно любой кусок текста отправить на аудит. Но, в том же Github это решает Gists.

urxvt ★★★★★
()
Ответ на: комментарий от static_lab

Например делали портал аналитики.

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

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

alex0x08 ★★★
() автор топика
Ответ на: комментарий от alex0x08

И сколько не видел проектов на ранней стадии - там везде треш и говнокод, какой смысл такое ревьюить не представляю.

Так может там как раз и говнокод, потому что с самого начала этот код нормально не ревьюили?

static_lab ★★★★★
()
Ответ на: комментарий от urxvt

Обычно, эти несколько человек ведут беседу, что уже не просто каша.

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

Честно говоря, я так и не понял, чем тебя традиционный подход не устраивает.

Не то чтобы не устраивает, но не хотелось бы повторять этот функционал в лоб в своей штуке )

alex0x08 ★★★
() автор топика
Ответ на: комментарий от alysnix

продукт уровня десктопной ос ваши несколько сеньоров смогут разработать за приемлемое время?

Нет. Но это и не обосновывает, почему ревьюить должен исключительно руководитель проекта или группы. Да и как вы себе представляете, что руководитель всего проекта оси будет ревьюить весь код?

static_lab ★★★★★
()
Ответ на: комментарий от static_lab

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

alysnix ★★★
()
Ответ на: комментарий от alex0x08

смысл ревью - выдать оценку а не вступать в переписку, нет?

Так комментарий к строке или блоку кода потенциально может быть началом диалога между автором кода и ревьюерами.

static_lab ★★★★★
()
Ответ на: комментарий от static_lab

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

alex0x08 ★★★
() автор топика
Ответ на: комментарий от alysnix

ревьюит конкретно код руководитель группы(типа бригадир) или доверенное лицо

Каково обоснование, что коллеги разработчика не могут его ревьюить?

static_lab ★★★★★
()
Ответ на: комментарий от alex0x08

Проекты же могут быть не только начатыми студентами и фрилансерами. Это просто у вас больше специализация именно на переписывании подобных проектов.

static_lab ★★★★★
()
Для того чтобы оставить комментарий войдите или зарегистрируйтесь.