Comments 3
Продублирую комментарии здесь, раз уж в ВК вы почему-то решили на них не отвечать :).
- Расширение зачем-то экранирует имена классов, утверждая, что это нужно для LSD, однако на самом деле LSD никак не препроцессит содержимое строк, которые отправляются, так что я предлагаю поправить описание или (ещё лучше) перестать экранировать то, что не просили :). Оставил комментарий на github про это. Также, если писать в файл в режиме append кусками меньше, чем 4 Кб, то такая запись будет атомарна, и в этом случае не требуется писать всё в разные файлы (но тогда нужно будет делать fflush() руками, если используется fopen/fwrite/fclose). С другой стороны, это будет генерировать много inotify событий, так что возможно действительно писать всё одним большим куском — это не такая уж и плохая идея, если хочется сэкономить CPU.
- Так а почему вы не пошли на шаг дальше и не добавляете логирование в неиспользуемые методы, чтобы потом окончательно убедиться в том, что они не используются и потом их точно также автоматически выпиливать? С помощью того же самого LSD и собирать эту статистику, просто напрямую из PHP-кода. Заодно не нужно плагины к шторму писать, поскольку это итак будет видно по первой же строке в методе, что это кандидат на удаление.
- По экранированию. Ты прав, что формулировка «нужно для LSD» была не совсем правильная с нашей стороны. Точнее надо было сказать «нужно было для нашей системы доставки сообщений, основанной на LSD». Самому демону это не надо, он доставит как есть. Но мы там можем доставлять сообщение не только в виде одной строки, но и в виде нескольких строк. Для этого мы экранируем при записи и ревертим при обработке и самое простое было сделать чтобы все клиенты писали одинаково.
Перестать экранировать можно опцией, если это кому-то надо будет, но пока просили мы сами себя как внутренние заказчики и потому она есть и выпиливать полностью пока не планируется. - По записи в файл. При записи маленьких кусочков будет атомарно. Но ты уже сам все тут ответил :) могу только подтвердить факт, который ты и так прекрасно знаешь, что для нас более важно экономия на CPU.
Плюс это было очень удобно для дебага посмотреть что пишется одним воркером и никто не обязывать использовать плейсхолдер для имени файла. Это просто опция :) - Возможно не до конца понял твой вопрос про автоматическое удаление неиспользуемых методов. Если мои ответы не о том, то дай знать пожалуйста. Вообще в целом мы не хотели автоматику по следующим причинам:
- Нет было задачи выпиливать все и сразу. Была задача помочь разработчику не снимая с него ответственности за чистоту кода
- Не хотелось захламлять код и историю репы автоматическим добавлением логирования в этот метод. Но рассматривали возможность делать это автоматом по требованию для конкретных случаев — пока не пригодилось, возможно поднимем вопрос еще раз позже
- Выпиливать код хочется логическими частями подобно тому как делаются логически завершенные изменения каждым коммитом, а не хаотичными автовыпилами. Как это сделать структурировано — я пока не представляю. Если есть мысли, то буду рад твоему совету.
Перестать экранировать можно опцией, если это кому-то надо будет, но пока просили мы сами себя как внутренние заказчики и потому она есть и выпиливать полностью пока не планируется.
Я просто хотел обратить внимание на то, что это экранирование не нужно для LSD и вероятно не нужно для кого-то ещё (или нужно в каком-то другом варианте), кто будет использовать это расширение, поэтому мне кажется, что было бы логичным выключить эту опцию по умолчанию, поскольку она сбивает с толку.
Плюс это было очень удобно для дебага посмотреть что пишется одним воркером и никто не обязывать использовать плейсхолдер для имени файла. Это просто опция :)
Если эту опцию не использовать, то из-за того, что при записи в файл fwrite буферизует запись блоками по сколько-то Кб, а не построчно, записи будут перемешиваться. То есть, если опция выключена, нужно убедиться в том, что буфер сбрасывается вручную не реже чем в N Кб, где N — это, допустим, 4 Кб на Linux, хотя в других операционных системах или даже реализациях libc может быть по-другому.
То есть, я хочу сказать, что если не хочется периодически получать мусор в этом файле, то эта опция обязательна в текущей реализации.
Нет было задачи выпиливать все и сразу. Была задача помочь разработчику не снимая с него ответственности за чистоту кода
Да, но если при этом возможно доказать, что этот метод и правда не используется, добавив логирование в продакшене и подождав ~полгода, почему бы и не выпиливать неиспользуемые методы автоматически :)?
Не хотелось захламлять код и историю репы автоматическим добавлением логирования в этот метод. Но рассматривали возможность делать это автоматом по требованию для конкретных случаев — пока не пригодилось, возможно поднимем вопрос еще раз позже
В целом, я согласен, но с другой стороны это нужно делать относительно редко, и присутствие строчки с логированием в методе позволило бы сразу видеть (без необходимости писать плагины к PHPStorm), что метод потенциально нигде не используется.
Выпиливать код хочется логическими частями подобно тому как делаются логически завершенные изменения каждым коммитом, а не хаотичными автовыпилами. Как это сделать структурировано — я пока не представляю. Если есть мысли, то буду рад твоему совету.
Мне кажется, эти подходы друг другу не противоречат. Мне кажется, если разработчики будут сами знать о том, что неиспользуемый код будет выпилен автоматически через какое-то время, они будут меньше бояться сами выпиливать ненужный код и делать это более «аккуратно» и продуманно, чем автоматика.
Что делать с легаси. Материалы с Badoo PHP Meetup #4