-
Notifications
You must be signed in to change notification settings - Fork 9
Установка версий 2.0* по номеру версии #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
## Walkthrough
В проект были внесены изменения, связанные с улучшением обработки версий и скачивания дистрибутивов OneScript. Добавлены новые экспортируемые функции для работы с семантическими версиями и получения списка дистрибутивов, обновлены проверки корректности версий, а логика выбора и скачивания файлов переработана для большей гибкости. В `.gitignore` добавлено исключение для файла настроек отладки.
## Changes
| Файл(ы) | Краткое описание изменений |
|------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------|
| .gitignore | Добавлено правило для игнорирования `.vscode/launch.json`. |
| src/core/Классы/ВерсииOneScript.os | Упрощена функция `Версия`, удалён второй параметр; удалён fallback-парсер HTML; функция `ПолучитьВерсииПоAPI` теперь возвращает новую таблицу и кидает исключения при ошибках; добавлена экспортируемая функция `ПолучитьДоступныеВидыДистрибутивовВерсии` для получения списка дистрибутивов по версии через API. |
| src/core/Классы/ПараметрыOVM.os | Удалена функция `МаскаНомераВерсии()`, добавлена экспортируемая функция `МаскаНомераВерсииSemver()`, возвращающая регулярное выражение, строго соответствующее спецификации semver. |
| src/core/Классы/УстановщикOneScript.os | Переименована функция проверки версии с `ЭтоТочныйНомерВерсии` на `ЭтоДопустимыйНомерВерсии` с использованием новой маски semver; переработана логика функции `ПолучитьПутьКСкачиваниюФайла` — теперь дистрибутив выбирается через поиск в списке доступных файлов с учётом типа дистрибутива, архитектуры и режима алиаса; добавлена новая функция `НайтиПодходящийДистрибутив`. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant Installer as УстановщикOneScript
participant VersionAPI as ВерсииOneScript
participant Params as ПараметрыOVM
User->>Installer: ПолучитьПутьКСкачиваниюФайла(версия, x64, fdd)
Installer->>Params: МаскаНомераВерсииSemver()
Installer->>Installer: ЭтоДопустимыйНомерВерсии(версия)
alt Версия допустима
Installer->>VersionAPI: ПолучитьДоступныеВидыДистрибутивовВерсии(версия)
VersionAPI->>VersionAPI: HTTP GET api/archive?all=true
VersionAPI->>VersionAPI: HTTP GET api/archive/<token>
VersionAPI-->>Installer: Таблица дистрибутивов
Installer->>Installer: Выбор подходящего дистрибутива
Installer-->>User: Ссылка на скачивание
else Версия невалидна
Installer-->>User: Исключение/Ошибка
endPossibly related PRs
Poem
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/core/Классы/УстановщикOneScript.os (1)
326-351:⚠️ Potential issueОпределение «версии 2» не работает для prerelease-тегов
Сравнение
СтроковыеФункции.СравнитьВерсии(ВерсияКУстановке, "2.0")упадёт, если в строке присутствует суффикс-preview,-rc1и т. д. – функция ожидает числовые части. В результате prerelease-сборки 2.0* будут обрабатываться как alias-версии, что нарушает логику выбора дистрибутива.Предлагаемый надёжный способ:
-ЭтоВерсия2 = СтроковыеФункции.СравнитьВерсии(ВерсияКУстановке, "2.0") >= 0; +// major >= 2 независимо от суффиксов +РВМажор = Новый РегулярноеВыражение("^(\d+)"); +ЭтоВерсия2 = (РВМажор.Совпадает(ВерсияКУстановке) + И Число(РВМажор.НайтиСовпадения(ВерсияКУстановке)[0].Группы[1].Значение) >= 2);Так мы корректно определим мажорную версию даже при наличии
-preview.
🧹 Nitpick comments (5)
src/core/Классы/ПараметрыOVM.os (1)
66-79: Добавить пометку устаревания и централизовать работу с масками версийТеперь в модуле присутствуют две функции, возвращающие регулярное выражение версии –
МаскаНомераВерсии()и новаяМаскаНомераВерсииSemver(). Поддержка двух разных масок без чёткой иерархии создаёт риск того, что разработчики будут случайно пользоваться упрощённой маской и получать «тихие» ошибки при работе с prerelease-тегами.Предлагаю:
- Явно пометить
МаскаНомераВерсии()как устаревшую (комментарием// DEPRECATEDили аналогичной аннотацией).- Во всех местах, где осталась старая маска, перейти на
МаскаНомераВерсииSemver().- В описании новой функции указать, что именно она является «единственным источником истины» по допустимому формату версии.
Функция МаскаНомераВерсии() Экспорт - Возврат "\d+\.\d+\.\d+(\.rc\d+?)?"; + // DEPRECATED: используйте МаскаНомераВерсииSemver() + Возврат "\d+\.\d+\.\d+(\.rc\d+?)?";Это позволит избежать дублирования логики и потенциальной рассинхронизации.
src/core/Классы/ВерсииOneScript.os (2)
264-299: Повторяющиеся константы и однотипная обработка ответов HTTPВ пределах одной функции дважды объявляется локальная переменная
HTTP_OK = 200, а блоки проверки статуса и формирования исключения практически идентичны (запрос кapi/archive?all=trueи затем кapi/archive/<token>). Это небольшое дублирование:
- Усложняет чтение;
- Увеличивает риск несогласованности изменений (например, если код состояния изменится или появятся дополнительные проверки заголовков).
Рекомендации:
-HTTP_OK = 200; +Перем HTTP_OK; // объявить однажды в начале функции +HTTP_OK = 200;и вынести проверку ответа в небольшую вспомогательную подфункцию внутри модуля:
Процедура ПроверитьОтвет(Ответ, ОписаниеОшибки) Если Ответ.КодСостояния <> HTTP_OK Тогда ВызватьИсключение СтрШаблон( "%1: Статус %2, Ответ: %3", ОписаниеОшибки, Ответ.КодСостояния, Ответ.ПолучитьТелоКакСтроку()); КонецЕсли; КонецПроцедурыТем самым код станет короче и поддерживать его будет легче.
278-285: Можно прервать перебор после нахождения первого совпаденияПосле того как найден
ИскомыйТокенВерсии, цикл продолжает проходить оставшиеся элементы массива, хотя это уже не требуется.Для Каждого ОписаниеВерсии Из МассивВерсий Цикл Если НомерВерсии = ОписаниеВерсии.presentation Тогда ИскомыйТокенВерсии = ОписаниеВерсии.token; + Прервать; КонецЕсли; КонецЦикла;Это не критично по производительности, но улучшит читаемость и устраняет лишние итерации.
src/core/Классы/УстановщикOneScript.os (1)
315-322: Сообщение об ошибке стоит сделать более информативнымСейчас при невалидной версии генерируется исключение с текстом, равным самой версии. Позднее по логам трудно понять причину.
-ВызватьИсключение ВерсияКУстановке; +ВызватьИсключение СтрШаблон("Недопустимый форм-фактор версии «%1»", ВерсияКУстановке);Это улучшит диагностику и юзабилити CLI-утилиты.
.gitignore (1)
2-2: Рассмотрите игнорирование всего каталога.vscode/
Чтобы автоматически исключать не толькоlaunch.json, но и любые другие локальные настройки VSCode (расширения, параметры и т.д.), можно игнорировать весь каталог:-.vscode/launch.json +.vscode/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.gitignore(1 hunks)src/core/Классы/ВерсииOneScript.os(1 hunks)src/core/Классы/ПараметрыOVM.os(1 hunks)src/core/Классы/УстановщикOneScript.os(3 hunks)
🔇 Additional comments (4)
src/core/Классы/ВерсииOneScript.os (1)
301-321: Уточнить типы колонок и защититься от отсутствия полей в JSONКолонки создаются без явного описания типов. При дальнейшей обработке может оказаться, что одно из полей
id,filename,arch,linkотсутствует (например, API изменилось), и тогда будет записаноНеопределено. Желательно:
- Задать тип
Строкадля всех колонок, чтобы на ранней стадии отловить несоответствия.- Перед добавлением строки проверять наличие полей.
-ТаблицаРезультата.Колонки.Добавить("Вид"); +ТаблицаРезультата.Колонки.Добавить("Вид", Новый ОписаниеТипов("Строка"));и аналогично для остальных колонок.
src/core/Классы/УстановщикOneScript.os (2)
307-312: Проверка семантической версии реализована корректноФункция аккуратно использует новую маску semver и не игнорирует регистр, что соответствует спецификации (пре-релизные и build-метки чувствительны к регистру). Замечаний нет.
379-380: Хорошо, что ссылка возвращается напрямуюВозврат прямой ссылки без дальнейших вычислений уменьшает связанность и упрощает тестирование. 👍
.gitignore (1)
2-2: Добавление локальной VSCode конфигурации в игнор
Добавление строки.vscode/launch.jsonв.gitignoreсоответствует лучшим практикам по исключению локальных настроек разработчика из репозитория.
src/core/Классы/ВерсииOneScript.os
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кандидат на распил в детальки, кстати...
src/core/Классы/ВерсииOneScript.os
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не понимаю, как текст исключения связан с тем, что делает функция. Вроде бы этот кусок просто получает все доступные версии, а есть ли среди них запрашиваемая или нет проверяется уже позже.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вся функция в целом ищет версию на сайте. Текст исключения относится к ней (может это только мне так кажется...)
src/core/Классы/ВерсииOneScript.os
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не обязательно в этот пр, но задумываюсь над тем, чтобы сюда 1connector затянуть
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/core/Классы/УстановщикOneScript.os (1)
359-385: Хорошая инкапсуляция логики поиска дистрибутиваФункция корректно реализует поиск с учётом всех нюансов:
- Фильтр по архитектуре применяется правильно (согласно API все типы, включая zip, имеют поле arch)
- Fallback логика для алиасов корректно сохраняет критерий архитектуры
Для улучшения читаемости можно добавить комментарий, объясняющий логику двойного поиска.
Рекомендую добавить пояснительный комментарий:
НайденныеСтроки = ПереченьФайлов.НайтиСтроки(КритерийПоиска); Если НайденныеСтроки.Количество() <> 1 Тогда - // Это может быть установка по алиасу и тогда есть шанс у дистрибутива zip для версии 1 + // При установке по алиасу (stable/dev) мы не знаем точную версию. + // Если платформенный дистрибутив не найден, пробуем универсальный zip (для v1.x) Если РежимАлиаса Тогда КритерийПоиска.Вид = "zip"; КонецЕсли;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/core/Классы/ВерсииOneScript.os(4 hunks)src/core/Классы/ПараметрыOVM.os(1 hunks)src/core/Классы/УстановщикOneScript.os(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/core/Классы/ПараметрыOVM.os
- src/core/Классы/ВерсииOneScript.os
🧰 Additional context used
🧠 Learnings (1)
src/core/Классы/УстановщикOneScript.os (2)
Learnt from: EvilBeaver
PR: oscript-library/ovm#41
File: src/core/Классы/УстановщикOneScript.os:358-364
Timestamp: 2025-05-22T17:24:02.028Z
Learning: При работе с API OneScript (oscript.io) для получения дистрибутивов, все типы архивов, включая `zip`, имеют корректно заполненное поле `arch` со значениями "x86" или "x64", поэтому фильтрация по архитектуре должна применяться ко всем типам дистрибутивов.
Learnt from: EvilBeaver
PR: oscript-library/ovm#41
File: src/core/Классы/УстановщикOneScript.os:366-377
Timestamp: 2025-05-22T17:19:26.197Z
Learning: В функции ПолучитьПутьКСкачиваниюФайла класса УстановщикOneScript при поиске дистрибутива через алиас сначала выполняется поиск по типу дистрибутива текущей платформы, а затем в случае неудачи - по типу дистрибутива "zip", при этом критерий архитектуры (x86/x64) должен сохраняться неизменным в обоих поисках.
🔇 Additional comments (2)
src/core/Классы/УстановщикOneScript.os (2)
307-312: Хорошее улучшение именования и централизация паттерна валидацииПереименование функции с
ЭтоТочныйНомерВерсиинаЭтоДопустимыйНомерВерсииболее точно отражает её назначение. Использование централизованного паттерна semver изПараметрыOVM.МаскаНомераВерсииSemver()улучшает поддерживаемость кода.
326-357: Отличный рефакторинг с улучшенной обработкой версийРефакторинг функции значительно улучшает надёжность:
- Трёхзначная логика для определения версии 2 корректно обрабатывает неопределённость при использовании алиасов
- Переход на API-подход вместо ручного построения путей повышает надёжность
- Явное разделение логики выбора типа дистрибутива улучшает читаемость
Хорошее архитектурное решение с делегированием поиска в отдельную функцию.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/core/Классы/УстановщикOneScript.os (1)
381-387: Отличное улучшение диагностики ошибокДетализированное сообщение об ошибке с указанием критериев поиска и количества найденных файлов значительно упрощает диагностику проблем при установке.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/core/Классы/УстановщикOneScript.os(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/core/Классы/УстановщикOneScript.os (2)
Learnt from: EvilBeaver
PR: oscript-library/ovm#41
File: src/core/Классы/УстановщикOneScript.os:358-364
Timestamp: 2025-05-22T17:24:02.028Z
Learning: При работе с API OneScript (oscript.io) для получения дистрибутивов, все типы архивов, включая `zip`, имеют корректно заполненное поле `arch` со значениями "x86" или "x64", поэтому фильтрация по архитектуре должна применяться ко всем типам дистрибутивов.
Learnt from: EvilBeaver
PR: oscript-library/ovm#41
File: src/core/Классы/УстановщикOneScript.os:366-377
Timestamp: 2025-05-22T17:19:26.197Z
Learning: В функции ПолучитьПутьКСкачиваниюФайла класса УстановщикOneScript при поиске дистрибутива через алиас сначала выполняется поиск по типу дистрибутива текущей платформы, а затем в случае неудачи - по типу дистрибутива "zip", при этом критерий архитектуры (x86/x64) должен сохраняться неизменным в обоих поисках.
🔇 Additional comments (3)
src/core/Классы/УстановщикOneScript.os (3)
307-312: Корректное использование semver маски для валидации версийПереименование функции и использование
МаскаНомераВерсииSemver()улучшает валидацию версий, позволяя корректно обрабатывать preview-версии и другие семантические версии.
315-315: Корректное обновление вызова переименованной функции
324-357: Отличная реализация гибкой логики определения версии и типа дистрибутиваТрёхзначная логика для
ЭтоВерсия2элегантно решает проблему неопределённости при работе с алиасами. Использование API для получения списка дистрибутивов вместо ручного конструирования путей значительно повышает надёжность.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кажется мы в этом случае при установке ovm i dev --fdd поставим таки scd а не fdd, потому что флаг мы обрабатываем только для "номерных" версий
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Да, исправил.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
nixel2007
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
|
@sfaqer у меня больше нет вопросов |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Сейчас:
В данном PR:
Summary by CodeRabbit
Новые возможности
Исправления ошибок
Рефакторинг
Прочее