Как стать автором
Обновить

Комментарии 48

Давайте я побуду практичным циником.

Зачем?
...
Если вы спросите своего лида нужен ли код-ревью, он ответит - конечно, потому что пропущенная на ревью ошибка, это его головная боль, задержанная в будущем задача или новая бага. С другой стороны, если вы подойдете с этим вопросом к продюсеру, особенно перед майлстоуном - да на agile он все эти ваши код-ревью вертел - неделя до сдачи, вагон тасок и два поезда багов - фичи надо пилить, а не в эпистолярном жанре упражняться и письма друг другу писать. Время разработчиков и деньги компании – ключевые вещи, если вы посмотрите на причины проведения код-ревью.
...

А где в этом всём польза (в деньгах) собственно разработчику? Если ревью уменьшает головную боль лида – пусть лид и занимается ревью.

Объясню: допустим, я, один из ревьюверов, заметил, что какой-то код "попахивает", непонятен или неоптимален. У меня два варианта: начать обсуждение и не начинать.

Допустим, я не начал, и просто нажал "одобрить". Через пол года возникла проблема. Если проблему спустили решать кому-то другому, то меня это вообще не касается. А если мне – то я могу героически решить эту проблему и, также героически порешав ещё пяток таких проблем, получить премию.

Теперь предположим обратное, и я начал обсуждение. Дальше опять же два варианта: первый "само программирование может быть связано с эмоциями", и на меня кто-то обиделся, или я добавил кому-то работы. Результат тут строго отрицательный, а я получу ровно такую же дополнительную работу в своём собственном ревью в следующий раз. Второй – коллеги мне сказали "спасибо", и, возможно написали в конце отчётного периода, что "Вася делает классные код-ревью". В этом случае результат околонулевой, потому что я не видел ещё ни одной компании, в которой бы этот показатель был хоть сколько-нибудь важным.

Итого: проделав анализ усилий и результата получаем, что кроме каких-то тривиальных случаев (типа указать товарищу на тривиальную опечатку, из-за которой он огребёт проблемный релиз на следующий день) всегда выгоднее одобрять ревью без особых замечаний.

Выдавать сразу качественную кодовую базу, минимизируя объем работы для QA, не? Представьте, как классно будет если ревьюверами будут условные 5 педантичных senior-маньяков с чувством перфекционизма и большим опытом чтения кода.

5 педантичных senior-маньяков с чувством перфекционизма и большим опытом чтения кода

Если они знают где Вы живете то боюсь у меня для Вас плохие новости.

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

Объём работы QA не сильно меняется. Более того, эти самые QA регулярно встречая пару ошибок оказываются очень полезными сотрудниками, и также получают бонусы за найденные проблемы в виде премий и "достижений" для повышения.

Опять же с позиции "практичного циника" я представляю, что в среднем вокруг средние люди. Возможно есть компании, у которых средний программист – это педантичный senior-маньяк с чувством перфекционизма и большим опытом чтения кода, но что-то мне подсказывает, что такие компании заканчивают как Sun Microsystems.

Уточню: я позицию "практического циника" озвучиваю в применении к позиции за зарплату. Естественно, если это фриланс, своя компания или опенсорс – эта позиция и анализ буду неприменимы.

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

А где в этом всём польза (в деньгах) собственно разработчику? Если ревью уменьшает головную боль лида – пусть лид и занимается ревью.

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

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

Какое обсуждение? Если видишь проблему, то написать пару строчек, что вот-тут проблема займет 5 минут от силы.

Через пол года возникла проблема. Если проблему спустили решать кому-то другому, то меня это вообще не касается. А если мне – то я могу героически решить эту проблему и, также героически порешав ещё пяток таких проблем, получить премию.

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

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

А на практике вот библиотека для скачивания и обработки больших данных спутниковой радарной съемки от сотрудника НАСА, написанная в рабочее время за зарплату: https://github.com/forrestfwilliams/burst2safe Сто килобайт (запутанного) кода на питоне, а в итоге, библиотека при каждом запуске скачивает данные заново, без проверки уже скачанных, а в случае ошибки при скачивании, естественно, прерывает работу. Когда нужно скачать пол терабайта данных - ошибки происходят, так что качать придется много раз. Ну и что, думаете, автор поправил ошибку после сообщения о ней? Нет, ему пофиг. При том, что адекватная реализация с проверкой скачанного в 100 строк кода доступна у меня в проекте PyGMTSAR. Может, под угрозой увольнения и поправит, а может, радостно пойдет в другую компанию работать так же. И таких примеров уйма. Если человек пишет негодный код - никакое ревью его не исправит, а вот автотесты, которые просто не дадут закоммитить код в основную ветку, проблему решают. Да, код останется аховым, но будет выполнять свою задачу, что намного лучше лучше даже хорошего кода, который не делает свою работу.

Если человек пишет негодный код - никакое ревью его не исправит

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

У нас можно поставит жёсткий блок на мердж. Пока тот кто поставил не снимет, вмерджить не выйдет, даже с апрувом от владельцев.

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

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

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

А где в этом всём польза (в деньгах) собственно разработчику?

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

Пример. Проблема: импорт данных стал занимать много времени после очередного релиза. Причина ясна и не может быть откатана назад (фикс другой очень важной проблемы). Я даю совет вместо импорта в базу по одной строчке использовать транзакции - человек не долго думаю оборачивает основной цикл (с походами в ФС и чтением ид3 тэгов включая формирование превью картинок из муз файлов и вызовом внутренних нотификаций/коолбэков об изменении таблиц) во, внимание, единственную транзакцию и рапортует о 100х улучшении.

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

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

У меня есть свое правило, я не начинаю дискашены на ревью до тех пор пока не увижу в МР первую критическую проблему, по типу как в примере или УБ коего в с++ чуть более чем везде и дофига много или если ПО накосячил с требованиями в тикете, что я уже вижу в коде (да, такое тоже бывает). После первый критической проблемы я добавляю уже и стилистические дискашены это не про форматирование, а про выбор способа сделать тоже самое (по сути), но другим синтаксисом, алгоритмом, структурой данных. Если крит проблем нет, то я апрувлю не смотря на то, что почти всегда могу накидать стилистических дискашенов ибо они найс ту фикс.

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

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

Не стоит гадать по юзерпику о СУБД все равно не угадаете:

  • В Скулайте все транзакции блокирующие, и не просто блокирующие, а полностью блокирующие все базы приаттаченные к коннекции

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

  • Во первых это не авто тест, а перформанс/бенчмарк тест. Разница серьезная. Не в коде теста, а в том где его запускать и как интерпретировать результат. Запускать такие тесты надо либо на однотипном железе либо на виртуалках с точными лимитами на ресурсы и строго изолированно (по ресурсам) иначе тесты будут флаки и просто удалены. Интерпретировать тоже просто так не выйдет нужна история запусков ибо если внезапно начало не проходить то это не 100% проблемы кода, чаще что то поменялось в окружении и надо чинить изоляцию или увеличивать тайм-аут.

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

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

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

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

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

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

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

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

Большую транзакцию и так заметят тестеры или юзеры, но позже конечно.

У меня на гитхабе есть расширения для эскулайт и легко нагуглить мои патчи двадцатилетней давности для эскулайт, например, для сжатия в модуле полнотекстового поиска. Увы, использовать эскулайт вы не умеете, но утверждаете, что там чего-то нельзя сделать! wal режим появился очень давно и очень хорош для большинства приложений. А разумная архитектура приложения с эскулайт - открыть базу данных в памяти, скопировать определения нужных таблиц из подключенной к ней основной базы, и дальше можно не спеша заполнять эти структуры таблиц (основную можно и отконнектить, если не нужна в данный момент), а потом подключить основную базу и очень быстро скопировать в нее с помощью команды COPY новые данные. Так вам не нужно создавать отдельные структуры данных в приложении и нет проблем с долговременными пишущими транзакциями на основную базу.
Вот простой пример на питоне с коллбэками на открытие базы и начало транзакции:

import sqlite3
def on_database_open():
    print("Database has been opened.")
def on_transaction_start():
    print("Transaction has been started.")
class CustomConnection(sqlite3.Connection):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        on_database_open()
    def cursor(self, *args, **kwargs):
        return CustomCursor(self, *args, **kwargs)
class CustomCursor(sqlite3.Cursor):
    def execute(self, sql, *args, **kwargs):
        if sql.strip().lower().startswith('begin'):
            on_transaction_start()
        return super().execute(sql, *args, **kwargs)
sqlite3.register_adapter(str, CustomConnection)
sqlite3.register_converter("CustomConnection", CustomConnection)
def connect_with_callback(db_name):
    return sqlite3.connect(db_name, factory=CustomConnection)
conn = connect_with_callback('example.db')
conn.execute('BEGIN TRANSACTION;')
conn.execute('CREATE TABLE IF NOT EXISTS test (id INTEGER PRIMARY KEY, name TEXT);')
conn.commit()
conn.close()

Как видите, эскулайт очень много чего умеет. Когда-то я делал и встроенный язык тикль в эскулайт - чтобы дампы PostgreSQL базы c триггерами и функциями на PL/pgTCL иметь возможность тестировать в эскулайт (компы были слабые и разворачивать инстанс постгреса с загрузкой дампа занимало часы).

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

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

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

В эскулайт нет такой проблемы - там есть отдельный wal файл, так что откат транзакции операция очень быстрая.

В тот раз я отделался "ай-яй-яй" и штрафом минус два дня к отпуску.

А это точно прописано в контракте и не нарушает законы?

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

Возможно вы скажете, что задача код-ревью это проверка изменений, а не способность написать короткое эссе - "как я пофиксил этим летом" к PR. Но подумайте, что описание комита это финализация задачи, и если вы не можете сформулировать в пяти-шести коротких предложениях, что конкретно было сделано, то есть повод задуматься, всё ли сделано в задаче что нужно.

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

А это точно прописано...

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

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

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

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

Первые лет десять я боялся бас-фактора. Но подхватив штук 10+ проектов без документации (а один раз и без коду, чисто джарки) я понял что бас-фактор ну очень сильно переоценен. Если код не полное говнище, то типичный сеньор расковыривает логику ну пусть за пару недель максимум. Хороший код вообще выглядит как "коридорный шутер" куда можно пускать даже джунов. В нем чисто на уровне архитектуры и интерфейсов больно делать неправильно.

А потом человек уходит в отпуск и два специалиста бросают свои дела на две недели, чтобы стабилизировать нестабильный сервер. А сервер хостит все репозитории компании и имеет полмиллиона запросов за 10 минут, в пике.

Если бы не это «если», то все верно :) На деле же, когда даже пиксели оказываются отдельными объектами, и массивы пикселей тоже объектами, и арифметические операции переопределены с помощью eval, а сверху еще таких же выкрутасов понавешено дюжиной слоев, то разобраться посложнее оказывается. Если что, пример вполне реальный - это известный софт анализа динамики океанов от NOAA на matlab. После переписывания на питоне «без выкрутасов» стал выполняться за две минуты на старом ноуте вместо двух суток на стоядерном кластере. Кстати, из наукоемкого там нашлось несколько десятков строк кода, решающих задачу линейной алгебры с помощью стандартной библиотеки, а все остальное - обертки над обертками (как вам десяток вложенных уровней eval?). Что интересно, и автотесты к исходному коду есть - но ни один релиз их не проходит, что никого не волнует, проверяют только, чтобы не крэшились.

Чисто ради интереса, я же правильно понимаю что вам никто эти знания не передавал? Сколько времени заняло это приключение?

Кроме форматеров есть ещё линтеры и статические анализаторы. Они отлавливают много вещей до ревью.

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

Упомянутая вами функция на четыре экрана будет далеко за пределами нормы.

Что-то я не припоминаю хороших инструментов, да и вообще хоть каких-то, для анализа сложности понимания и восприятия кода.

Sonar вполне справляется с оценкой коплексности кода, например, вложенность if-else, for, количестао строк и т.п.

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

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

Это в какой стране можно такие контракты делать?

Эстония, Кипр, Италия, Мальта, Испания, Штаты (с остальными не сталкивался) в РФ свои приколы, особенно на госе

Про Эстонию сильно сомневаюсь. Работал там полтора года и ничего похожего не слышал.

У него все эти принципы разработки качественного софта выбиты ноликами и единицами поверх серого вещества.

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

Вот так подраматичнее будет)

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

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

Игровые движки часто перекочевывают из проекта в проект в рамках одной студии.

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

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

возраст отдельных кусков кода движка

Звучит так, что "долгоживущий код" -- это редкость.

я не беру сторонние либы

А зачем их брать? Или вы их тоже ревьювите?

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

Так если вы форкнули стороннюю либу, то это уже ваша либа.

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

Например, переименовать тестовый класс согласно пункта 5 сглашения 'ссылка'. Где прописано, что тестовые классы именуются так-то, т.к. применяется маска и т.п. (Такую проверку, конечно, можно настроить в статическом анализаторе. Просто не во всех проектах возможно под себя перенастроить правила анализатора).

Это сильно сокращает время ревью и дает возможность посмотреть развернутое описание. Конечно, соглашение возможно пересматривать, дополнять, вносить исключения, но только аргументировано.

Зарегистрируйтесь на Хабре, чтобы оставить комментарий

Публикации

Истории