манипуляция провайдерами настроек через совет другого мастера#139
Open
Segate-ekb wants to merge 3 commits intoautumn-library:masterfrom
Open
манипуляция провайдерами настроек через совет другого мастера#139Segate-ekb wants to merge 3 commits intoautumn-library:masterfrom
Segate-ekb wants to merge 3 commits intoautumn-library:masterfrom
Conversation
Co-authored-by: Copilot <copilot@github.com>
WalkthroughВводится новый ленивый построитель конфигурации параметров Changes
Sequence Diagram(s)sequenceDiagram
participant Поделка as Поделка
participant СДМ as СоветДругогоМастера
participant Строитель as СтроительМенеджераПараметров
participant Метапров as ФайловыйМетапровайдер
participant МП as МенеджерПараметров
Note over Поделка,МП: Новый поток (после PR)
Поделка->>СДМ: ПолучитьСтроительМенеджераПараметров()
activate СДМ
СДМ->>Строитель: ленивое создание/возврат кэша
activate Строитель
Строитель->>МП: создать/получить МенеджерПараметров()
activate МП
Строитель->>МП: добавить env-провайдер (приоритет 1)
Строитель->>Метапров: создать файловый метапровайдер (форматы: yaml/json/ini)
activate Метапров
Метапров->>МП: зарегистрировать физические провайдеры для каждого формата
Метапров-->>Строитель: метапровайдер готов
deactivate Метапров
Строитель->>МП: добавить inline/прочие провайдеры
deactivate МП
Строитель-->>СДМ: вернуть построитель
deactivate Строитель
SДМ->>Строитель: применить устаревшие настройки, если явно заданы
Строитель->>МП: обновить конфигурацию (при необходимости)
SДМ-->>Поделка: вернуть построитель
deactivate СДМ
Поделка->>Строитель: МенеджерПараметров()
Поделка->>МП: Прочитать()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
docs/api/050-Классы/25-СтроительМенеджераПараметров.md (1)
221-221: Небольшая языковая правка для естественности формулировки.Фразу можно сделать чище: “Проксирует настройки сразу во все физические провайдеры”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/api/050-Классы/25-СтроительМенеджераПараметров.md` at line 221, Заменить фразу в описании, чтобы звучала естественнее: изменить текст "Проксирует настройки сразу в физические провайдеры (`<база>-yaml`, `<база>-json`, `<база>-ini`)" на "Проксирует настройки сразу во все физические провайдеры (`<база>-yaml`, `<база>-json`, `<база>-ini`)" в блоке, описывающем методы ДобавитьПровайдерФайлов / ПровайдерФайлов.tests/СтроительМенеджераПараметров.os (1)
128-146: Добавьте проверку расширения.ymlв тест определения формата.Сейчас в этом сценарии закреплены
.yaml/.json/.ini, но не.yml. Имеет смысл добавить отдельный кейс, чтобы не потерять поддержку альтернативного YAML-расширения при будущих изменениях.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/СтроительМенеджераПараметров.os` around lines 128 - 146, В тесте Процедура ДобавитьПровайдерФайлНастроекОпределяетФорматПоРасширению добавьте ещё один вызов Строитель.ДобавитьПровайдерФайлНастроек с путём, оканчивающимся на ".yml" и отдельную проверку Ожидаем.Что(Строитель.Провайдер("yml")).Не_().Равно(Неопределено) и проверку, аналогичную НастройкиY.ПутьКФайлуПараметров, чтобы убедиться, что формат для .yml определяется корректно; используйте существующие методы Провайдер и Настройки() для точной локализации изменений.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/api/050-Классы/25-СтроительМенеджераПараметров.md`:
- Line 9: В документации заменить некорректный вызов метода
СоветДругогоМастера.СтроительМенеджераПараметров() на актуальный
СоветДругогоМастера.ПолучитьСтроительМенеджераПараметров() в примерах
(упоминания в строках с вызовом и ссылках, напр. на строках с вызовом и в
анкорах), чтобы пример соответствовал реалиям API и тестам; проверьте и обновите
связанные ссылки/якоря (например
20-СоветДругогоМастера.md#СтроительМенеджераПараметров) так, чтобы они указывали
на ПолучитьСтроительМенеджераПараметров.
In `@packagedef`:
- Line 19: The package definition currently depends on an unpublished configor
version (.ЗависитОт("configor", "0.12.0")), which makes installs fail; update
the dependency to the latest published version by changing the .ЗависитОт call
for "configor" to use "0.11.1" (or revert to "0.12.0" later once it is
published) so the packagedef is resolvable.
In `@src/internal/Классы/СтроительМенеджераПараметров.os`:
- Around line 161-168: УдалитьПровайдер сейчас только вызывает
Метапровайдер.Отключить() и удаляет запись из _ФайловыеМетапровайдеры, но не
удаляет физические базовые провайдеры в _МенеджерПараметров, поэтому повторный
ДобавитьПровайдерФайлов("default",...) вызывает коллизии и старые обёртки в
ФайловыйМетапровайдер.ПриСозданииОбъекта(); исправьте УдалитьПровайдер так,
чтобы после Метапровайдер.Отключить() также удалялись все связанных с этим
метапровайдером физические провайдеры из _МенеджерПараметров (например по id или
по ссылке), либо вместо удаления просто пересобирали/реинициализировали
_МенеджерПараметров без удаляемого провайдера, чтобы при следующем
ДобавитьПровайдерФайлов не осталось старых записей и не возникли коллизии
идентификаторов.
In `@src/internal/Классы/ФайловыйМетапровайдер.os`:
- Around line 111-118: УстановитьФайлПараметров сейчас передаёт один и тот же
ПутьКФайлу каждому провайдеру из коллекции Провайдеры, что ломает multi-format
сценарии (например одинаковый путь попадает в yaml, json и ini провайдеры);
исправьте это либо запретив вызов УстановитьФайлПараметров когда в Провайдеры
зарегистрировано более одного формата (кинуть ошибку/лог), либо изменить логику
так, чтобы УстановитьФайлПараметров не распределял путь по всем провайдерам и
оставить возможность задавать конкретный путь только через
ДобавитьПровайдерФайлНастроек(…) — внесите проверку количества/типа провайдеров
в функцию УстановитьФайлПараметров и действуйте согласно выбранной политике.
In `@src/Классы/СоветДругогоМастера.os`:
- Around line 44-76: Функция ПолучитьСтроительМенеджераПараметров всегда создаёт
новый экземпляр, из‑за чего внешние изменения строителья теряются; исправьте это
в классе СоветДругогоМастера, сохранив строитель в поле (например,
this.СтроительМенеджераПараметров) и возвращая существующий экземпляр при
повторных вызовах вместо всегда Нового; при первом вызове создайте и настройте
Строитель как сейчас, затем кешируйте его; обновите также сценарий
ИнициализироватьМенеджерПараметров/Поделка, если он ожидал делиться тем же
объектом, и добавьте регрессионный тест, который берёт строитель из
СоветДругогоМастера, модифицирует провайдеры и затем передаёт
СоветДругогоМастера в Поделка, ожидая сохранить изменения.
---
Nitpick comments:
In `@docs/api/050-Классы/25-СтроительМенеджераПараметров.md`:
- Line 221: Заменить фразу в описании, чтобы звучала естественнее: изменить
текст "Проксирует настройки сразу в физические провайдеры (`<база>-yaml`,
`<база>-json`, `<база>-ini`)" на "Проксирует настройки сразу во все физические
провайдеры (`<база>-yaml`, `<база>-json`, `<база>-ini`)" в блоке, описывающем
методы ДобавитьПровайдерФайлов / ПровайдерФайлов.
In `@tests/СтроительМенеджераПараметров.os`:
- Around line 128-146: В тесте Процедура
ДобавитьПровайдерФайлНастроекОпределяетФорматПоРасширению добавьте ещё один
вызов Строитель.ДобавитьПровайдерФайлНастроек с путём, оканчивающимся на ".yml"
и отдельную проверку
Ожидаем.Что(Строитель.Провайдер("yml")).Не_().Равно(Неопределено) и проверку,
аналогичную НастройкиY.ПутьКФайлуПараметров, чтобы убедиться, что формат для
.yml определяется корректно; используйте существующие методы Провайдер и
Настройки() для точной локализации изменений.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b767da23-3a06-440d-9c69-b14878bf8386
📒 Files selected for processing (10)
docs/api/050-Классы/20-СоветДругогоМастера.mddocs/api/050-Классы/25-СтроительМенеджераПараметров.mddocs/api/index.mdpackagedefsrc/internal/Классы/СтроительМенеджераПараметров.ossrc/internal/Классы/ФайловыйМетапровайдер.ossrc/Классы/Поделка.ossrc/Классы/СоветДругогоМастера.ostests/СоветДругогоМастера.ostests/СтроительМенеджераПараметров.os
| .ЗависитОт("annotations", "1.4.0") | ||
| .ЗависитОт("asserts", "1.4.0") | ||
| .ЗависитОт("configor", "0.9.1") | ||
| .ЗависитОт("configor", "0.12.0") |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Опубликована ли версия пакета configor0.12.0 в реестре пакетов OneScript и доступна ли она уже для установки изpackagedef?
💡 Result:
Нет, версия пакета configor 0.12.0 не опубликована в реестре пакетов OneScript и не доступна для установки из packagedef. Последняя доступная версия на GitHub - v0.11.1 (от 2026-02-17), а в зависимостях других проектов указываются версии до 0.11.0. Пакет configor доступен для установки командой opm install configor, но конкретно 0.12.0 не найдена ни в releases репозитория https://github.com/oscript-library/configor, ни на hub.oscript.io.
Citations:
Версия configor 0.12.0 не опубликована в реестре пакетов
Последняя доступная версия configor в реестре OneScript — v0.11.1 (от 2026-02-17). Использование неопубликованной версии 0.12.0 делает этот packagedef нерезолвимым при попытке установки. Требуется либо дождаться выпуска версии 0.12.0 в реестр, либо временно указать последнюю доступную версию (0.11.1).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packagedef` at line 19, The package definition currently depends on an
unpublished configor version (.ЗависитОт("configor", "0.12.0")), which makes
installs fail; update the dependency to the latest published version by changing
the .ЗависитОт call for "configor" to use "0.11.1" (or revert to "0.12.0" later
once it is published) so the packagedef is resolvable.
Comment on lines
+161
to
+168
| Процедура УдалитьПровайдер(Знач Идентификатор) Экспорт | ||
|
|
||
| Метапровайдер = _ФайловыеМетапровайдеры.Получить(Идентификатор); | ||
|
|
||
| Если Метапровайдер <> Неопределено Тогда | ||
| Метапровайдер.Отключить(); | ||
| _ФайловыеМетапровайдеры.Удалить(Идентификатор); | ||
| Возврат; |
There was a problem hiding this comment.
После УдалитьПровайдер() файловый метапровайдер нельзя корректно добавить заново с тем же id.
Тут удаление сводится к Отключить(), но физические <base>-yaml/json/ini остаются зарегистрированными в _МенеджерПараметров. При повторном ДобавитьПровайдерФайлов("default", ...) получите коллизии идентификаторов, а ФайловыйМетапровайдер.ПриСозданииОбъекта() потом заберёт старые обёртки по исходным id. В итоге сценарий "удалить/заменить провайдер" работает некорректно. Нужно либо реально удалять физические провайдеры из менеджера, либо пересобирать менеджер без удаляемого провайдера.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/internal/Классы/СтроительМенеджераПараметров.os` around lines 161 - 168,
УдалитьПровайдер сейчас только вызывает Метапровайдер.Отключить() и удаляет
запись из _ФайловыеМетапровайдеры, но не удаляет физические базовые провайдеры в
_МенеджерПараметров, поэтому повторный ДобавитьПровайдерФайлов("default",...)
вызывает коллизии и старые обёртки в ФайловыйМетапровайдер.ПриСозданииОбъекта();
исправьте УдалитьПровайдер так, чтобы после Метапровайдер.Отключить() также
удалялись все связанных с этим метапровайдером физические провайдеры из
_МенеджерПараметров (например по id или по ссылке), либо вместо удаления просто
пересобирали/реинициализировали _МенеджерПараметров без удаляемого провайдера,
чтобы при следующем ДобавитьПровайдерФайлов не осталось старых записей и не
возникли коллизии идентификаторов.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Классы/СоветДругогоМастера.os (1)
57-176:⚠️ Potential issue | 🟠 MajorКешированный строитель не синхронизируется с поздними legacy-сеттерами.
Сейчас legacy-настройки попадают в
_Строительтолько при первом создании. Если кто-то уже получил строитель, а потом вызоветЗначенияДеталек(...)/ПрефиксПеременныхСреды(...)/ИмяФайлаСоЗначениямиДеталек(...), cached instance останется со старыми значениями. Это делает порядок вызовов публичного API наблюдаемым и ломает обратную совместимость.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Классы/СоветДругогоМастера.os` around lines 57 - 176, Кешированный _Строитель в функции ПолучитьСтроительМенеджераПараметров не синхронизируется с устаревшими сеттерами (ЗначенияДеталек, ПрефиксПеременныхСреды, ИмяФайлаСоЗначениямиДеталек, ДополнительныйКаталогПоискаФайлаСоЗначениямиДеталек): если _Строитель уже создан, последующие вызовы этих методов не влияют на него; исправьте это либо инвалидируя кеш (_Строитель = Неопределено) внутри каждого legacy-сеттера после установки значений, либо (лучше) сразу применять изменения к уже существующему строителю — получить текущий Строитель через ПолучитьСтроительМенеджераПараметров() и обновить провайдеры/метапровайдер (вызовы Провайдер("env").Настройки().УстановитьПрефикс, УдалитьПровайдер("inline")/ДобавитьПровайдерСоответствие, ПровайдерФайлов("default").УстановитьИмяФайла/УстановитьСтандартныеКаталогиПоиска) чтобы состояние всегда оставалось согласованным.
🧹 Nitpick comments (1)
tests/СоветДругогоМастера.os (1)
180-230: Добавьте кейс для позднего вызова legacy-сеттера.Текущие тесты покрывают только сценарий “сначала выставили устаревшую настройку, потом создали строитель”. Если кеш строителя остаётся частью контракта, стоит зафиксировать и обратный порядок: сначала
ПолучитьСтроительМенеджераПараметров(), потом legacy-сеттер, чтобы было ясно, должен ли кеш обновляться или этот сценарий больше не поддерживается.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/СоветДругогоМастера.os` around lines 180 - 230, Добавьте тесты, проверяющие поведение при позднем вызове устаревших сеттеров: создайте новый тест (например ПрименитьУстаревшиеНастройкиПрефиксENV_ПослеПолученияСтроителя и аналогичные для ИмяФайлаСоЗначениямиДеталек и ЗначенияДеталек) где сначала вызывается Совет.ПолучитьСтроительМенеджераПараметров(), затем вызываются Совет.ПрефиксПеременныхСреды("MYAPP") / Совет.ИмяФайлаСоЗначениямиДеталек("my-config") / Совет.ЗначенияДеталек(Значения) и после этого проверяется, обновился ли кеш/строитель: читать из Строитель.Провайдер(...).Настройки().ПолучитьПрефикс() или Строитель.Провайдер("default-json").Настройки().ПолучитьНастройки() и Менеджер = Строитель.МенеджерПараметров(); Менеджер.Прочитать(); Ожидаем.Что(Менеджер.Параметр("ключ")).Равно("значение") — чтобы зафиксировать ожидаемое поведение при позднем сеттинге.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Классы/СоветДругогоМастера.os`:
- Around line 182-189: Измените текст в процедуре ПредупредитьОбУстаревшемМетоде
класса СоветДругогоМастера, чтобы не предлагать «передавая строитель в
конструктор Поделки» (это неверно для текущего публичного API). Переформулируйте
предупреждение так, чтобы рекомендовать использовать
СтроительМенеджераПараметров при создании или конфигурации самого
СоветДругогоМастера (или явно указать правильный точка интеграции), упомянув
правильные символы: СоветДругогоМастера, Поделка и СтроительМенеджераПараметров,
чтобы пользователи получили корректную инструкцию по миграции.
---
Outside diff comments:
In `@src/Классы/СоветДругогоМастера.os`:
- Around line 57-176: Кешированный _Строитель в функции
ПолучитьСтроительМенеджераПараметров не синхронизируется с устаревшими сеттерами
(ЗначенияДеталек, ПрефиксПеременныхСреды, ИмяФайлаСоЗначениямиДеталек,
ДополнительныйКаталогПоискаФайлаСоЗначениямиДеталек): если _Строитель уже
создан, последующие вызовы этих методов не влияют на него; исправьте это либо
инвалидируя кеш (_Строитель = Неопределено) внутри каждого legacy-сеттера после
установки значений, либо (лучше) сразу применять изменения к уже существующему
строителю — получить текущий Строитель через
ПолучитьСтроительМенеджераПараметров() и обновить провайдеры/метапровайдер
(вызовы Провайдер("env").Настройки().УстановитьПрефикс,
УдалитьПровайдер("inline")/ДобавитьПровайдерСоответствие,
ПровайдерФайлов("default").УстановитьИмяФайла/УстановитьСтандартныеКаталогиПоиска)
чтобы состояние всегда оставалось согласованным.
---
Nitpick comments:
In `@tests/СоветДругогоМастера.os`:
- Around line 180-230: Добавьте тесты, проверяющие поведение при позднем вызове
устаревших сеттеров: создайте новый тест (например
ПрименитьУстаревшиеНастройкиПрефиксENV_ПослеПолученияСтроителя и аналогичные для
ИмяФайлаСоЗначениямиДеталек и ЗначенияДеталек) где сначала вызывается
Совет.ПолучитьСтроительМенеджераПараметров(), затем вызываются
Совет.ПрефиксПеременныхСреды("MYAPP") /
Совет.ИмяФайлаСоЗначениямиДеталек("my-config") / Совет.ЗначенияДеталек(Значения)
и после этого проверяется, обновился ли кеш/строитель: читать из
Строитель.Провайдер(...).Настройки().ПолучитьПрефикс() или
Строитель.Провайдер("default-json").Настройки().ПолучитьНастройки() и Менеджер =
Строитель.МенеджерПараметров(); Менеджер.Прочитать();
Ожидаем.Что(Менеджер.Параметр("ключ")).Равно("значение") — чтобы зафиксировать
ожидаемое поведение при позднем сеттинге.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea1ce1df-9476-4014-b4d9-3d8a55f0ba63
📒 Files selected for processing (5)
docs/api/050-Классы/25-СтроительМенеджераПараметров.mdsrc/internal/Классы/ФайловыйМетапровайдер.ossrc/Классы/СоветДругогоМастера.ostests/СоветДругогоМастера.ostests/СтроительМенеджераПараметров.os
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/СтроительМенеджераПараметров.os
- src/internal/Классы/ФайловыйМетапровайдер.os
Comment on lines
+182
to
+189
| Процедура ПредупредитьОбУстаревшемМетоде(Знач ИмяМетода, Знач РекомендуемаяЗамена) | ||
|
|
||
| Лог.Предупреждение(СтрШаблон( | ||
| "Метод СоветДругогоМастера.%1(...) устарел." | ||
| " Используйте СтроительМенеджераПараметров.%2," | ||
| " передавая строитель в конструктор Поделки.", | ||
| ИмяМетода, РекомендуемаяЗамена | ||
| )); |
There was a problem hiding this comment.
Уточните текст предупреждения об устаревшем API.
В сообщении сказано “передавая строитель в конструктор Поделки”, но по текущему публичному API Поделка по-прежнему создаётся из СоветДругогоМастера. Сейчас warning может увести пользователей на неверный путь миграции.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Классы/СоветДругогоМастера.os` around lines 182 - 189, Измените текст в
процедуре ПредупредитьОбУстаревшемМетоде класса СоветДругогоМастера, чтобы не
предлагать «передавая строитель в конструктор Поделки» (это неверно для текущего
публичного API). Переформулируйте предупреждение так, чтобы рекомендовать
использовать СтроительМенеджераПараметров при создании или конфигурации самого
СоветДругогоМастера (или явно указать правильный точка интеграции), упомянув
правильные символы: СоветДругогоМастера, Поделка и СтроительМенеджераПараметров,
чтобы пользователи получили корректную инструкцию по миграции.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Требует oscript-library/configor#49
Если вкратце:
Инкапсулировал билдер менеджера параметров в отдельной абстракции.
Теперь есть возможность управлять настройками гибко. Переопределять приоритеты чтения, добавлять, изменять и удалять провайдеры.
Пример: