Pull to refresh

Comments 69

То есть когда я юности писал что-то вроде
if ($_SERVER['REMOTE_ADDR'] != '127.0.0.1')
error_reporting(E_ALL ~E_WARNING)

, это я, оказывается, feature toggle делал. Буду знать=)
Фича тогглы это такое жуткое зло, лучше уж мерж. Они превращают проблему одного человека в проблему всей команды. При мало-мальски недисциплинированных разработчиках проект все время в «красном» состоянии. Зачастую он даже не компилируется. И CI работает лишь как индикатор — появился коммит, загорелся сигнал, что мастер сломан. Где то тут на хабре даже кто то уличный светофор приволок в офис, чтобы этот сломанный мастер сразу было видно. Очевидно, ломается он нередко. И надо об этом быстро узнать и подорваться и чинить. И разработчики все время стараются свалить на кого то вину. «Это он своим коммитом меня сломал, у меня все работало!!!»

С большими мержами надо бороться другими способами. Во первых код должен быть хорошо декомпозирован. При этом два разработчика работающие над двумя разными фичами не будут пересекаться, собственно потому что фичи декомпозированы друг от друга. Нет пересечений — нет мержей. Кроме этого чисто математически — чем больше файлов, тем меньше вероятность пересечься в одном файле. Во вторых фичи должны быть маленькие. Руби коммьюнити для нас всех придумали отличный инструмент для этого. Надо заставить аналитика писать gherkin тесты, и один тест — одна фича, а не как ему «на берегу» кажется, 100500 фич, вроде бы друг к другу относятся, тем более одни от других зависят, дай ка я их оформлю в одну историю. Фичи, которые быстро разрабатываются, очевидно, исключают долгоживущие ветки.
И в случае веток, CI это не индикатор, это защитник. Он не дает вмержить «красную» ветку, поэтому мастер всегда зеленый. Нерадивый разработчик не может даже теоретически свалить вину на кого то. Если твоя ветка красная — виноват, очевидно, только ты.
Фича тогглы это такое жуткое зло, лучше уж мерж. Они превращают проблему одного человека в проблему всей команды. При мало-мальски недисциплинированных разработчиках проект все время в «красном» состоянии.

А как "проект все время красный" вытекает из feature toggles? По-моему так ровно наоборот.


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

Это пока у вас фичи ядро не затрагивают.


Кроме этого чисто математически — чем больше файлов, тем меньше вероятность пересечься в одном файле.

Проблемы мержа возникают не только тогда, когда правки в одном файле.


Надо заставить аналитика писать gherkin тесты, и один тест — одна фича

А что такое "один тест"? Он покрывает и положительные, и отрицательные сценарии?


а не как ему «на берегу» кажется, 100500 фич, вроде бы друг к другу относятся, тем более одни от других зависят, дай ка я их оформлю в одну историю.

Есть такой нюанс: иногда есть фичи, которые по отдельности маленькие, но выпускать (т.е., показывать пользователю) можно только вместе. И да, вмерживать их надо (и можно) по отдельности, а вот чтобы пользователь их не видел, помогают… внезапно, feature toggles.

А как «проект все время красный» вытекает из feature toggles? По-моему так ровно наоборот.
Анекдот такой был — в пьянстве замечен не был, но по утрам жадно пил холодную воду. Для одних светофор в офисе дает сигнал, что мастер надо чинить, для меня он дает сигнал, что фича тогглы — зло.

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

Вы, наверное, и на улице такие же выводы делаете? Раз уж светофор красный?


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

А я вот вижу проект без feature toggles, и все равно периодически слышу, что мастер не собирается. Так что же зло тогда?

Думаю, имелось в виду, что зло не фича тогглы, а процесс, когда все подряд коммитят в мастер. А вот когда все коммитят в разные ветки, то они ломают только свои ветки. А в мастер мержат только зелёные ветки, поэтому мастер, как правило, зелёный.

Когда дважды прямым текстом написано "Фича тогглы это такое жуткое зло" — сложно догадаться, что имеется в виду что-то другое.

Когда на мастере правильные проверки в pre-merge hook, его не сломать мержем ветки. А если вдруг всё-таки сломают, то code freeze до стабилизации.

С долгоживущими ветками сложнее сделать быстрый и безопасный релизный цикл.

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

И это как раз тот случай, когда в проекте с фича тогглами «опять упал мастер», а в случае с ветками все хорошо.

Неа. Мастер упал в обоих случаях, просто в одном (с feature toggles) — рано (что хорошо), а в другом (без них) — поздно (что плохо).

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

Ну так это от feature toggle никак не зависит, либо есть такой барьер, либо нет.


Ах да, только что ты говорил, что мастер не падает с фича тогглами

Где?

Фаст-форвард тут совсем не причём. Хоть с ним, хоть без него может оказаться в мастере наследник без реализации только что появившегося абстрактного метода.

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

ЗЫ К слову, этот пример еще и есть нарушение принципа Open-Close. Просто говорю.

А если не повесил? Интерпретируемый язык без 100% реального (а не построчного) тестового покрытия. Или компилируемый даже, но в CI реальной компиляции нет, только линтинг.

UFO just landed and posted this here
Главное условие использование фича-свитчей — хорошее тестовое покрытие, в идеале полное. Если нет ни компиляции ни тестов — только ветки, мерж или не мерж, выбора просто нет.
Неа. Мастер упал в обоих случаях, просто в одном (с feature toggles) — рано (что хорошо), а в другом (без них) — поздно (что плохо).
Нет, мастер не падает, если работаем с ветками. Когда разработчик вмержит в свою ветку мастер, то увидит, что оно не собирается.
Когда разработчик вмержит в свою ветку мастер, то увидит, что оно не собирается.

Если.

Что значит «если»? Если разработчик не сделал мердж, не проверил, ветка не прошла ревью и вмержил, и сломал этим мастер, то этот разработчик саботажник и с ним будет проведен серьёзный разговор

Ну вот это все очень хорошо и приятно слышать. Но если. Потому что не везде работает.

Это само собой. Я один раз, много лет назад, вообще работал в фирме интересно. Заходил на продакшн сервер через FTP, на котором играли игроки, напряму в js-файлы вносил изменения и просил по скайпу основной костяк игроков нажать ф5, чтобы проверить, пропал ли баг и ничего ли не сломалось.

Но мы ведь не будем брать такие процессы в виде здоровых примеров и контраргументов?

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

Feature toggling — это необязательно инструмент организации совместной разработки. Зачастую это способ внедрения бизнес-задач, запуск которых не зависит от разработчика.


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

Разумный пример, и всё-таки включение/выключение на уровне кода основного приложения выглядит костыльно. После отключения фичи, если она одноразовая — её надо выпилить из проекта, тогда придётся заводить опять ветку в которой все эти if удаляются.
В идеале, эта функциональность должна включаться зависимостью на уровне системы сборки, и там же выключаться убрав какой-нибудь compile 'company:fifa-extension:1.0' из build.gradle например. А определение места, куда «внедрить» функциональность должна быть на стороне библиотеки-расширения. Навскидку правда пока не могу вспомнить/придумать механизма работы этого

Фичетоггл в проекте — фактически запрет на рефакторинг. Не то, чтобы он совсем стал невозможен, но усложняется на порядок.


Так что — спасибо, нет.


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

А откуда в мастере возьмется актуальный код, если все ветки долгоживущие?

Если все ветки долгоживущие, надо что-то в консерватории менять.


Обычно их немного — это какие-то сложные фичи, которые не бьются на подзадачи.


И, да, под долгоживущими я понимаю "дольше недели".

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

Да даже трех-четырех (а я регулярно вижу намного больше одновременно разрабатываемых "сложных фич") достаточно, чтобы устроить ад во время общего мержа.

Да не надо дожидаться общего мержа! Ребейзить себе потихоньку.


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

Да не надо дожидаться общего мержа! Ребейзить себе потихоньку.

На что ребейзить-то? Еще раз: у вас есть три крупных фичи, долгоживуших. Чтобы вам было, на что ребейзиться, нужно, чтобы эти фичи начали мержить в мастер рано; а "рано" в случае долгоживущей фичи -это "до завершения". В свою очередь, чтобы замержить фичу до завершения, вам надо как-то так сделать, чтобы она, хоть и была замержена, никак не влияла на поведение. И вот тут-то переключатели и помогают.

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


Допустим, в одной фиче отрефакторили класс, выделив часть методов в отдельный. Куда тут переключатели, простите, засунуть?

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

Вот, один вмержил, другие отребейзились и поправили свой код. Все нормально же!

Забыли один нюанс: у него долгоиграющая фича, он не может так просто взять и вмержить. Если он вмержит сейчас как есть, в мастер попадет его фича, которая еще не готова к выпуску, и всем испортит настроение.

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


А в фичеветке останется то, что собственно к фиче и относится, ну и ок.


Вот читаю я это все и создается впечатление, что git-ом пользуются как svn-ом, ну нельзя же так.

Рефакторинг прекрасно черрипикается в отдельную ветку.

Или нет. Если повезло — черрипикается. А если не повезло — нет.

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


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

Одну смержили, остальные отребейзили,

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


Понятно же, казалось бы, что feature toggles — они не на каждый день и не на каждую задачу, но бывают задачи, которые они решают успешно.

Да нет никаких незаконченных. Что закончили, то и вмерджили.

Ну вот поэтому и ждут до дедлайна, потому что фичу до того самого дедлайна делать и будут. А рефакторинг — это не фича, он отдельной ценности не имеет.

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

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

Если фичи пересеклись физически — должна упасть сборка проекта. Если они пересеклись логически — должны упасть тесты. Если тесты не упали, а проект не работает, то что то с этим проектом пошло не так.

Код может не работать и еще по массе причин.


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


У меня вообще правило: каждый раз перед тем, когда фичеветку "передаю" кому-то другому (на тестирование, на ревью), делаю rebase на мастер и прогоняю тесты. Если вижу, что в мастер чего-то намерджили существенное — тоже делаю rebase.

Так ведь ещё одна проблема — огромный пуллреквест, при долгоживущей ветке.

Решается interactive rebase-ом всей фичеветки, с перегруппировкой коммитов по принципу atomic commits, в "естественном" для чтения порядке.


Такое вполне удобно ревьюить по одному коммиту последовательно.

Я бы вот так делать не стал.


if (configurationManager.getParameter("is.some.functionality.enabled")) {
    // do some stuff
} else {
    // do default logic
}

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


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


Например


if (featureProvider.isFeatureEnabled("JIRA-TICKET-1234")) {
    registerEverywhere(SomeBrandNewFeature.class);
}

или


if (featureProvider.isFeatureEnabledInContext(context, "JIRA-TICKET-4321")) {
    executionPipeline = executionPipeline.next(SomeBrandNewFeature::doSomething);
}

И еще хочу добавить, что состояние хорошей фичи проверяется не более чем в одном месте. 2-3 условия — уже повод задуматься о рефакторинге. Если же проверки разбросаны по десяткам файлов, то фичетогглинг будет только мешать.

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

В общем пока непонятно когда это может быть лучше чем просто долгоживущая фичеветка
В общем пока непонятно когда это может быть лучше чем просто долгоживущая фичеветка

Когда вы не хотите постоянно тратить время на актуализацию этой фичеветки.

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

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

Ну уж, с таким подходом и тестировщики не нужны, если полагаться только на автотесты. Однако я пока такой сценарий плохо представляю. Тесты не гарантируют работоспособность (как и тестировщики конечно, но это последний и самый «интеграционный» уровень защиты). Тесты только могут гарантировать НЕработоспособность

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

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

Я бы вот так делать не стал.

Такие ветвления вносят неразбериху.

Непустая ветка else еще не самое худшее, что може произойти. У нас на портале визард. В JSON лежит конфигурация, степы в порядке следования, дочерние степы и т.п. Разработчик добавляет новый степ. Пока не придумали ничего лучше, чем по if грузить один JSON или другой. Теперь представляем как два разработчика в параллель добавляют два разных степа. Что случится если включить сразу обе фичи? появятся все нужные степы в визарде? очевидно нет. json содержащий все нужные степы просто не существует.
UFO just landed and posted this here
Участвовал я в проекте, где практиковался такой подход. Самая жесть начинается, когда фича признается ненужной до того как её полноценно включают. Если это мелкая локальная фича, то все еще нормально, удалил код и дело с концом. А если это более-менее сложная фича, то для неё делались какие-то рефакторинги (которые могут быть еще не закончены), где-то менялось API для этой фичи (при этом эти изменения могла делаться на скорую руку, ибо нет уверенности, что фича действительно уйдет в прод), и вот это все уже не откатишь нормально. В итоге в мастере скапливаются «хвосты» от кучи таких фичей, а архитектура постепенно обрастает костылями.
Как раз сейчас тоже думаю о долгоживущих ветках. Что если новая фича это большая переработка старой фичи. В этом случае в основной develop ветке будет жить старый код фичи/новый код фичи/feature toggle.
1) Во первых это накладывает дополнительный гемор для работы. При этом ничего не мешает сломать еще и старый код…
2) В конце концов нужна будет «последняя» итерация по выпиливанию старого кода и самой feature toggle
Плюсы feature toggle пока не вот что бы очевидные.
Во первых это накладывает дополнительный гемор для работы. При этом ничего не мешает сломать еще и старый код…
Это, кстати первое, что бросилось в глаза, когда я начал работать в проекте со свитчами. Нельзя просто так выпилить старую более никому не нужную логику, и по завершению фичи должен работать и новый код и старый. Нельзя выпилить старую колонку в базе. Это добавляет кучу лишней работы. И новый код приходится писать так, чтобы он не конфликтовал со старым, т.е. кроме того, что старый код надо продолжать поддерживать, в новом коде появляются технические решения, которые не объяснить требованиями. И в будущем, когда приходится еще раз потрогать этот код, идешь к разработчику, который последний раз его трогал и спрашиваешь, почему так, а он отвечает фразой — «исторически так сложилось».
Нельзя просто так выпилить старую более никому не нужную логику, и по завершению фичи должен работать и новый код и старый. Нельзя выпилить старую колонку в базе.

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

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

Вот только я не про код писал. Иногда, впрочем, надо сохранять и старые интерфейсы тоже, а интерфейсы — это, в общем-то, код.

Интерфейсы — это контракт. За точно таким же контрактом может стоять какой угодно код.


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

Интерфейсы — это контракт. За точно таким же контрактом может стоять какой угодно код.

Контракт, выраженный в коде, по крайней мере, в некоторых средах разработки.


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

Вот только нет никакого "снаружи", интеграционный код напрямую с БД работает. Плохо? Да. Можно поменять? Нет.

Я все же предлагаю рассматривать разумный дизайн ПО.


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


Ну, в общем, я понял, фиче-тогглы — это такая штука для тесно связанного лапшекода, в котором хуже уже не станет, потому что и так особо некуда. Я так и догадывался. :-)

Счастливые люди, у которых нет задачи обратной совместимости.
И несчастливые те, которые сами себе ее придумали, там где в ней не было необходимости.
Там где отключаемые фичи требует бизнес, там не откажешся. Например показывать один компонент базовым пользователям и другой компонент премиум пользователям. Но чтоб на свою голову придумывать сложности…
И самый эпик, наверное, где есть и те механизмы и эти. Если фича включена и пользователь базовый — компонент 1, если пользователь премиум — компонент 2, если фича выключена, то соответственно компоненты 3 и 4. Веселье.
И несчастливые те, которые сами себе ее придумали, там где в ней не было необходимости.

Ну, об этом вы с ними поговорите, не со мной.


И самый эпик, наверное, где есть и те механизмы и эти. Если фича включена и пользователь базовый — компонент 1, если пользователь премиум — компонент 2, если фича выключена, то соответственно компоненты 3 и 4. Веселье.

Типичное такое A/B-тестирование.

Sign up to leave a comment.

Articles