search

webasyst / webasyst-framework

Webasyst PHP Framework
http://www.webasyst.com/
GNU Lesser General Public License v3.0
290 stars 202 forks source link

Add missed waSystemConfig::$application #345

Closed WinterSilence closed 1 year ago

WinterSilence commented 2 years ago

https://github.com/webasyst/webasyst-framework/blob/bc24e06859c5fb82764d5aea80925845c437f488/wa-system/config/waSystemConfig.class.php#L561

mikeushenin commented 2 years ago

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

WinterSilence commented 2 years ago

@mikeushenin Михаил, что Вам не понятно во фразе Add missed waSystemConfig::$application? PR 2 строки...

mikeushenin commented 2 years ago

@mikeushenin Михаил, что Вам не понятно во фразе Add missed waSystemConfig::$application? PR 2 строки...

Во фразе всё понятно. Но вы не ответили на мой вопрос.

WinterSilence commented 2 years ago

@mikeushenin это и есть ответ на Ваш вопрос. В waSystemConfig используется свойство $application, объявляемое только в его потомке waAppConfig.

mikeushenin commented 2 years ago

@mikeushenin это и есть ответ на Ваш вопрос. В waSystemConfig используется свойство $application, объявляемое только в его потомке waAppConfig.

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

WinterSilence commented 2 years ago

@mikeushenin Михаил, я рассказал уже более чем достаточно. Просто Вы за эти годы так и не освоили программирование, поэтому общение с Вами напоминает разговор с ребенком и это очень утомляет т.к. ответов понятных не программистам тут не может просто быть.

Leonix commented 2 years ago

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

Если для вас, Антон, важнее статистика вашего профиля, чем принести пользу опен-сорсному проекту, то я прошу вас никогда больше не создавать пул-реквестов в репозиториях Вебасист. Да, я понимаю, как важно для фрилансера иметь портфолио и производить впечатление на клиентов. К счастью, на гитхабе достаточно других проектов. На ваш век хватит заполнить ленту активности.

Если же вы хотите по правде принести пользу, то я прошу вас переориентировать образ мышления.

Люди вокруг вас не глупы. Люди вокруг вас умеют читать код и понимают, что значит перенести поле из дочернего класса в родительский. И когда Миша просит вас обосновать это изменение и уточнить мотивы к этому действию, то это не потому, что Миша не смог прочитать код. А совсем по другим причинам. Я сейчас постараюсь, как умею, объяснить эти причины.

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

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

Что в данном случае? Риски - велики: затронуты два фундаментальных системных класса в пул-реквесте от человека, которому не очень свойственно задумываться о последствиях. Предполагаемая польза - сомнительна: в самом лучшем случае ничего не сломается, никто ничего не заметит. Естественный вопрос: зачем это нужно? Потому что одно дело, если "в самом лучше случае ничего не сломается", и совсем другое, если это изменение, например, лечит фатал еррор при попытке авторизации во фронтенде.

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

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

WinterSilence commented 2 years ago

@Leonix я бы понял Ваши притензии, если бы всё не было столь прозрачно - заголовок недвусмысленно дает понять цель PR, в описании PR указана строка, где используется отсутствующий параметр, а сам PR из 2х строк.