Установка версий 2.0* по номеру версии#41
Merged
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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соответствует лучшим практикам по исключению локальных настроек разработчика из репозитория.
nixel2007
reviewed
May 22, 2025
nixel2007
reviewed
May 22, 2025
Member
There was a problem hiding this comment.
Кандидат на распил в детальки, кстати...
nixel2007
reviewed
May 22, 2025
Member
There was a problem hiding this comment.
Не понимаю, как текст исключения связан с тем, что делает функция. Вроде бы этот кусок просто получает все доступные версии, а есть ли среди них запрашиваемая или нет проверяется уже позже.
Member
Author
There was a problem hiding this comment.
Вся функция в целом ищет версию на сайте. Текст исключения относится к ней (может это только мне так кажется...)
nixel2007
reviewed
May 22, 2025
Member
There was a problem hiding this comment.
Не обязательно в этот пр, но задумываюсь над тем, чтобы сюда 1connector затянуть
nixel2007
reviewed
May 22, 2025
nixel2007
reviewed
May 22, 2025
nixel2007
reviewed
May 22, 2025
There was a problem hiding this comment.
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.
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 для получения списка дистрибутивов вместо ручного конструирования путей значительно повышает надёжность.
sfaqer
requested changes
May 23, 2025
Member
There was a problem hiding this comment.
Кажется мы в этом случае при установке ovm i dev --fdd поставим таки scd а не fdd, потому что флаг мы обрабатываем только для "номерных" версий
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Member
|
@sfaqer у меня больше нет вопросов |
sfaqer
self-requested a review
May 23, 2025 23:34
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
3 participants
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.
Сейчас:
В данном PR:
Summary by CodeRabbit
Новые возможности
Исправления ошибок
Рефакторинг
Прочее