Привет, Игнат! Расскажи немного о себе
Я инженер-программист, работаю в CleverLabs. В основном пишу на руби, но в целом писал но почти всех актуальных языках программирования. Мне нравится ковырять вещи :)
На каком ты сейчас проекте работаешь?
Работаю над приложением для рынка почтовых пересылок. Агрегатор провайдеров, где провайдер на выбор предоставляется пользователю. И трекер посылок, который позволяет видеть жизненный цикл посылки почти-в-реальном-времени. Полгода уже работаю над ним.
Проект состоит из двух ruby приложений и одного мобильного приложения. API — входная точка без базы, на grape. Там находятся зависимости от внешних сервисов. Второе приложение — бэкэнд на rails, построенный на Engines, изолированных друг от друга. Два фронтенда - web и mobile. Vue.js и Flutter для IOS и Android.
Как ты начинал в нем разбираться?
На рельсовой части обычно я сразу смотрю контроллеры, но здесь их нет :) Потому что все находится в отдельных engines, смотрел уже их. Рельсовые модели у нас супер-легкие, без валидаций, только со связями. Для того, чтобы использовать модель, от нее нужно наследоваться и расширять для своих нужд. Соответственно все наследуемые модели лежат в подпапках в моделях. Это наша реализация DDD — не совсем честно DDD, но похоже на то. Получается, что доменные области мы моделируем на основе базы.
А так рельсовые проекты довольно одинаковые. Прежде всего смотрю на контроллеры, затем модели. Еще мне нравится смотреть в папку lib/
, где, как правило, хранятся полу-хаки. Такие прикольные штуковины, которые в проекте придумали, и они распространяются на всё остальное. То есть там можно увидеть, что в проекте есть такого нестандартного. Как правило, все расширения стандартного фреймворка уходят в lib/
— вроде переопределений или money-patching. Потом уже смотрю JS, какие-то детали ещё.
Ты стараешься сам разбираться в проекте, или узнаешь у людей?
Если вопрос связан с бизнесом — то спрашиваю у людей. Если это вопрос в реализации, то мне интересно самому разобраться. Стараюсь понять, как люди пришли к полученному решению, почему сделано именно так. Если совсем недоволен решением, то буду спрашивать. Как правило, странному решению есть какое-то объяснение. Было какое-то допущение перед созданием, и на основе него уже пришли к определенному решению. В худшем случае тебе скажут, что просто не было времени.
Когда понял, что знаешь проект?
Он довольно маленький, так что когда прочитал весь ruby код.
Есть ли документация в проекте?
Техническая документация только базовая — что используем, что поддерживаем, пару UML-диаграмм. Сам я этим не особо пользуюсь. Оно классно, когда приходишь на проект, а когда проект знаешь — то не сильно полезно.
Что главное при code-review? На что смотришь?
На противоречия. Насколько содержание кода соответствует тому, что в нем написано. И насколько написанное совпадает с тем, как оно работает.
Например, у тебя есть юзеры. Ты делаешь users.map(&:emails)
, и получились user emails. И часто бывает, что имя результата не то, неправильно описывает результат. Такое бывает на уровне методов, классов, больших конструкций. Чем сложнее структура данных, тем тщательней я смотрю на имена, которые получаются в результате работы с этой структурой.
Вот еще пример: есть юзеры активные, есть удаленные. Если делаешь .filter(&:active)
, это должно быть active_users
, а не filters
, или там not_inactive_something
. Потому что потом это название может идти дальше вглубь, и будут несовпадения. Или ты передаешь в метод count_users
массив employees
— что-то явно не так, не совпадает назначение метода, и что ты туда передаешь. И так бывает со всем.
Мне не нравится, когда люди идут на компромиссы, чтобы не менять лишний код. Типо: "ну нормально, будет понятно из контекста". А он нифига не работает, потому что впоследствии что-то меняется, и данное имя уже не совсем актуальное. Код должен делать то, что он говорит. И это, пожалуй, одна из главных вещей, которые я смотрю.
Второе это потенциальные проблемы. То есть смотришь на вещи, которые уже сталкивался кучу раз, очевидные для тебя. Например Null safety.
Еще важно смотреть наличие сидов. Чтобы можно было запустить у себя таску и проверить.
Как разбираешься с кодом в pull request'е?
Начинаю со входной точки. Смотрю как данные перетекают, и их весь путь. То есть в контроллер пришли данные, потом в какой-то сервис, там перемололись, отформатировались, ушли обратно.
В быстром код-ревью по порядку быстро просматриваю файлы. Pull request'а посерьёзнее смотрю более детально, использую поиск использований классов на странице. Когда большой pull request, то клонирую код и смотрю у себя
Если я могу вместить код у себя в голове, то можно посмотреть быстро. То есть "считал" pull request, увидел там три файла, то значит смотрю быстро.
Делаешь ли пометки по pull request'у?
Все пометки пишу в pull request. Если есть какой-то вопрос, то пишу сразу комментарий. Но пишу так, чтобы человека не обидеть. Пытаюсь узнать, какие у него были намерения. Например: "Возможно в этом месте нужно сделать create!
, потому что дальше мы нигде не делаем проверку на создание". Это нормально работает, люди отвечают. Если что-то сложное, то окей, пойдем обсудим. Но, как правило, хватает комментариев.
Фичей "просмотрено" в GitHub не пользуюсь. Мне кажется, что она бесполезная. Можно, конечно, скрыть какой-нибудь css-файл, чтобы он не мозолил глаза. Но для всего остального не пользуюсь. Не могу скрыть файл, потому что часто ищу использования классов, и со скрытием поиск по странице не работает.
Смотришь ли на задачу?
Зависит от pull request'а. Если он маленький, то не смотрю. Но если в нем файлов 10 и больше, то смотрю. Есть определенная граница, по которой я отсекаю простые ПР, на доверии автору. В маленькой задаче не в чем ошибиться, в большой уже больше деталей, и нужно смотреть.
Есть прикольная практика, когда разработчики записывают видео результата. "Вот такая задача, вот так она была сделана". То есть тебе не нужно идти на UI и париться, работает ли это вообще концептуально.
Насколько часто ты клонируешь код pull request'а?
Где-то 15% pull request'ов клонирую. Как правило, задачи — это эпик, который разбит на таски. Сами задачи маленькие, и смотреть код по ним просто. Но задачу, которая объединяет компоненты, точно нужно клонировать, даже если смотрел код до этого. Потому что бывает проблемы интеграции разных частей кода, какие-то данные не перетекают куда нужно, например.
Очень важны в ревью пробелы :). Чем больше отступы — тем больше вложенность. Есть такая шутка, что людям нравится писать вложенный код, потому что есть врожденная любовь к пейзажам гор :)
Что используешь для чтения кода?
В основном читаю код только в Sublime text. В нём есть классный поиск, который стабильно работает.
У гита есть глобальный конфиг, и глобальный .gitignore
. У меня в таком глобальном .gitignore
прописана папка _knowledge_base
, в которой я храню всякую информацию по проекту. Такая папка у меня есть в каждом проекте. В неё уходит вся инфа, которая не вошла в readme или wiki, и которая потом может пригодиться.
Как смотришь Open source проекты?
Первым делом readme. Затем скорее всего вики. Есть два типа OS — заматеревшие, у них читаю официальные и иногда даже независимые вики. Вторые - молодые, с нестабильным API. В них читаю документацию. Но когда этого не хватает — то уже лезу в код.
Бывает такое, что в readme описывает базовые примеры, а вики описывает уже частные случаи. Например во flutter в каждом проекте есть папка examples, и в ней описаны готовые базовые использования этой библиотеки.
А как читаешь код Open source проектов?
В начале я пытаюсь понять, что мне надо. Если какой-то метод, то начинаю с него . Потом есть Sourcegrapgh, он может делать поиск в публичных и не только репозиториях. Или поиск по GitHub использую — смотрю как другие люди пользуются этим методом в своих репозиториях. Там часто много всякого мусора, конечно, но в целом довольно классно, видно как люди организовывают использование инструмента.
Иногда в проекте есть публичный API, а есть "хитрый" публичный, не задокументированный. Он как бы есть, но может в любой момент отвалиться. И всегда тут нужно решать: "а могу ли я использовать?".
Для кода самое простое — склонировать репозиторий. Клонирую, и начинаю идти "вверх" от того, что мне нужно. А потом оттуда снова иду "вниз". "Вверх" — это во входную точку. Чтобы понять, где начинается выполнение кода, и какие данные наслаиваются к конечному моменту. Чтобы увидеть весь граф зависимостей как бы от информации.
Рисуешь что-нибудь на бумажке?
Постоянно. Если вложенность больше, например, пяти. На свежий могу держать в уме больше абстракций, но в конце дня нужно уже чаще записывать на бумагу для нормального понимания. Самое классное, это когда я пишу какие классы со входными данными. Но бывает, что хватает просто цепочки вызовов функций. И иногда приходится писать зависимости, например для Dependency Injection.
Как выбираешь NPM-пакет или Gem
Прежде всего я смотрю, могу ли я его не использовать.
На этапе прототипирования (для MVP например) - беру пакет ближайший, который попался и работает. По мере развития уже смотрю, насколько я этому пакету доверяю.
На доверие влияет дата последнего комита, но не всегда — иногда пакет один раз сделали, и он работает, на удивление без изменений. Это редко, но бывает. Смотрю так же открытые issue, pull request'ы. Смотрю на сам код, насколько он банально "опрятный". Если в библиотеке подключен хотя бы какой-нибудь code linter, то это уже классно. Значит создатели немного заботятся о том, что происходит, хотя в идеале должен быть работающий CI. Если все соблюдается — значит это нормальный пакет.
Иногда я могу почитать, и быть несогласным с архитектурой библиотеки. Но это окей, если сам по себе код чистый.
Насколько часто смотришь на чистоту библиотеки?
Есть пакеты маленькие, типо GitHub клиента octokit. А есть большие, например devise. Весь devise я не посмотрю, хотя впоследствии я уже знаю, что он так себе. Но т.к. это де-факто стандарт, все его знают, и он быстро добавляется в проект, то я его использую. А если библиотека какая-то небольшая, то его можно посмотреть уже саму.
Иногда я просто создаю main.rb
файл, пишу inline bundle
(таким образом гемы ставятся в папку проекта), пытаюсь что-нибудь накидать. Смотрю как это смотрится в плане моего взаимодействия с библиотекой, насколько мне комфортно. Бывает такое, что у тебя есть какой-то гем, он норм работает, но у него ужасный интерфейс. Например метод возвращает результат в виде "error23"
, и ты вообще не понимаешь, что это за ошибка, и почему 23.
Но, к сожалению, не всегда есть подходящая замена библиотеке, и иногда на крайний случай я беру и форкаю, подпиливаю. Ну и всегда можно написать обертку над кривой библиотекой, и её вызывать. Другим разработчикам и мне хотя бы будет комфортно вызывать его из кода.
Чего тебе сейчас не нравится в чтении кода?
Когда коменты на китайском :) В проекте мало чего не нравится, потому что есть rubocop, он более или менее стандартизирует. Иногда не хватает описания предположений, которым придерживались люди, ушедшие из проекта. Бывает такое, что ты не понимаешь, почему какие-то решения на каких основаниях были сделаны. Этого не хватает.
В целом мне не нравится неряшливость. Ты видишь, что можно сделать что-то лучше довольно просто. Но иногда люди забивают на такой мелкий рефакторинг. Вот отдаст на него человек полчаса, но зато потом сэкономит всем время.
А что нравится?
Мне нравится, чтобы в итоге получался такой код, при виде которого я понимаю, почему он так написан.
Что бы ты хотел посоветовать людям?
Есть интересная идея от Shopify. У них таски не слишком большие, и когда человек пишет код, первый раз реализовывает, потом делает git reset --hard
, и пишет её с нуля. То есть с контекстом ты смотришь на реализацию более аккуратно. Иногда я так тоже делаю.