-
Notifications
You must be signed in to change notification settings - Fork 2
Поддержка semver в части пререлизов и метаданных билда #7
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
Уточнена версия зависимости от fluent Добавлены workflow тестирования и публикации релиза Репозиторий подчищен от неактуальных файлов
@coderabbitai review |
✅ Actions performedReview triggered.
|
""" WalkthroughВ проект добавлены и обновлены файлы для поддержки чтения и сравнения версий с pre-release и build metadata. Внесены изменения в классы парсинга и сравнения версий, добавлены новые тесты, обновлены workflow-файлы CI/CD, а также изменены зависимости и структура тестовых сценариев. Changes
Sequence Diagram(s)sequenceDiagram
participant Тест as Тест
participant Версия as Класс Версия
participant Парсер as ПарсерВерсии
Тест->>Версия: Создать("1.2.3-alpha+build.5")
Версия->>Парсер: Инициализация парсера с версией
Парсер->>Версия: Возвращает токены (число, точка, число, точка, число, дефис, текст, плюс, текст, точка, число)
Версия->>Версия: Разбирает prerelease и build metadata
Тест->>Версия: ВСтроку()
Версия-->>Тест: "1.2.3-alpha+build.5"
Assessment against linked issues
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 4
🔭 Outside diff range comments (1)
src/core/Классы/Версия.os (1)
97-105
:⚠️ Potential issueНекорректное сравнение релизной и предрелизной версий
Согласно спецификации SemVer, стабильная версия без пререлиза (
1.0.0
) всегда считается старше любой версии с пререлизом (1.0.0-alpha
,1.0.0-rc.1
, …).
Условие на строках 97–101 проверяет эту ситуацию только для случаев, когда у входящей версии больше одного компонента пререлиза (> 1
). Если же пререлиз состоит из одного компонента (самый распространённый случай —alpha
,beta
,rc
), управление переходит вСравнитьПререлизы
, где пустой массив текущей версии приводит к возврату-1
. В результате релизная версия ошибочно считается меньше.Предлагаемый фикс:
- Если ПреРелиз.Количество() = 0 - И ВходящаяВерсия.ПреРелиз.Количество() > 1 Тогда + Если ПреРелиз.Количество() = 0 + И ВходящаяВерсия.ПреРелиз.Количество() > 0 Тогда Возврат 1;Также стоит добавить модульный тест, который явно сравнивает
1.0.0-alpha
и1.0.0
.
🧹 Nitpick comments (3)
tasks/coverage.os (1)
25-29
: Проверьте, что методИмяФайлаСтатистики()
корректно берёт файл по умолчаниюВы убрали явный аргумент (раньше передавался путь), теперь генератор должен сам искать
stat.json
в рабочем каталогеout
. У большинства версийcoverage
-плагина это действительно дефолт, но если в проекте используется более старая версия, отчёт может не собраться.- .ИмяФайлаСтатистики() + .ИмяФайлаСтатистики("stat.json") // во избежание сюрпризовsrc/core/Классы/ПарсерВерсии.os (1)
54-106
: Обработчик разделителей не потребляет символ → потенциальный бесконечный циклПосле попадания на разделитель цикл прерывается, но переменная
Индекс
не изменяется, еслиЧислоСимволов = 0
.
Ниже, в ветке «это разделитель», символ читается черезСред(...)
, а затемИндекс
инкрементируется – всё хорошо.
Тем не менее убедитесь, что ни один другой путь выхода из цикла не оставляет разделитель непрочитанным, иначе при следующем вызовеПолучитьСледующий()
вы получите тот же токен и зациклитесь.src/core/Классы/Версия.os (1)
127-129
: Семантически понятнее использовать<>
вместоНЕ … =
Запись
НЕ Сравнить(...) = 0
читается сложнее, чем эквивалентнаяСравнить(...) <> 0
. Предлагаю заменить для лучшей читаемости:- Возврат НЕ Сравнить(ВходящаяВерсия) = 0; + Возврат Сравнить(ВходящаяВерсия) <> 0;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/workflows/qa.yaml
(1 hunks).github/workflows/testing.yml
(1 hunks)packagedef
(2 hunks)sonar-project.properties
(1 hunks)src/core/Классы/Версия.os
(7 hunks)src/core/Классы/ПарсерВерсии.os
(1 hunks)src/core/Модули/Версии.os
(1 hunks)tasks/coverage.os
(1 hunks)tests/preReleasesTests.os
(1 hunks)tests/versionParserTest.os
(1 hunks)tests/Версии_test.os
(4 hunks)
🔇 Additional comments (10)
sonar-project.properties (1)
1-6
: Конфигурация SonarQube настроена правильноФайл конфигурации содержит все необходимые параметры для анализа кода: идентификатор и название проекта, пути к исходному коду и тестам, кодировка и путь к отчету о покрытии. Структура соответствует стандартам SonarQube.
packagedef (2)
7-7
: Обоснованное повышение основной версииИзменение версии с 0.6.0 на 1.0.0 корректно отражает значимость внесенных изменений в соответствии с семантическим версионированием. Поддержка пререлизов и метаданных билда - это существенное функциональное дополнение, которое заслуживает увеличения мажорной версии.
18-18
: Добавлена зависимость для анализа покрытия кодаДобавление зависимости от пакета "coverage" версии 0.7.0 соответствует внедрению интеграции с сервисами анализа качества кода и согласуется с новыми workflow-файлами.
src/core/Модули/Версии.os (1)
47-54
: Оптимизировано определение максимальной версииФункция
МаксимальнаяИзМассива
была переработана на более прямолинейную реализацию без использования внешних зависимостей. Новая реализация корректно находит максимальную версию путем последовательного сравнения элементов массива.Это изменение:
- Устраняет зависимость от библиотеки fluent
- Делает код более явным и понятным
- Сохраняет функциональную эквивалентность предыдущей реализации
.github/workflows/qa.yaml (1)
1-15
: Настроен процесс контроля качества кодаДобавлен GitHub Actions workflow для анализа кода с помощью SonarQube и интеграции с Coveralls. Конфигурация корректна и использует переиспользуемый workflow из репозитория autumn-library/workflows.
Этот workflow дополняет существующую конфигурацию CI/CD и обеспечивает автоматизированный контроль качества кода при каждом изменении.
tasks/coverage.os (1)
8-10
: Переезд выводов покрытия в каталогout
выглядит корректноКаталог создаётся заранее, путь к статистике прописан единообразно – проблем не вижу.
tests/Версии_test.os (3)
8-10
: Аннотация&Тест
улучшает авто-поиск тестовДобавление директивы делает тесты совместимыми с
oneunit
и избавляет от ручного реестра. 👍
40-42
: Мелкий стиль: логирование без лишних разделителей
СтрСоединить(..., " ")
уже вставляет пробелы, поэтому"%1"
будет выглядеть читаемо. Всё ок.
54-55
: Проверьте порядок аргументов вПроверитьРавенство
Вы поменяли местами ожидаемое и фактическое значение. В большинстве реализаций asserts именно такой порядок и используется (
Ожидаемое
,Фактическое
), но в старых версиях библиотеки было наоборот. Если CI ранее проходил, убедитесь, что после изменения тесты всё ещё зелёные.tests/versionParserTest.os (1)
38-41
: Проверьте, не нужен ли дополнительный возврат EOFТри последовательных вызова
Парсер.Следующий()
после достижения конца строки проверяют идемпотентность метода, что полезно. Однако при изменении парсера этот контракт может стать излишне строгим и привести к ложным срабатываниям. Подумайте, достаточно ли двух вызовов (первый выдаётТипКонецТекста
, второй — удостоверяемся, что повторный вызов не падает).
[ suggest_nitpick ]
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: 0
♻️ Duplicate comments (1)
src/core/Классы/Версия.os (1)
228-228
: Дублирование функциональности в методах чтенияКак было отмечено в предыдущих комментариях, методы
ПрочитатьПреРелиз
иПрочитатьМетаданныеСборки
имеют очень похожую логику. Также в коде есть упоминания методаПрочитатьОчереднойРазделитель
, который не представлен в коде, но может быть связан сПрочитатьРазделитель
.Рекомендуется рассмотреть возможность объединения дублирующейся логики в параметризованные методы для уменьшения объема кода и упрощения поддержки.
Also applies to: 281-286, 288-293
🧹 Nitpick comments (1)
src/core/Классы/Версия.os (1)
228-247
: Метод для обработки разделителейМетод
ПрочитатьРазделитель
корректно обрабатывает ожидаемые разделители в строке версии и выбрасывает исключение с подробным описанием ошибки при несоответствии.Однако, логика проверки на наличие разделителя может быть упрощена.
- Если ТекущийТокен.Тип = Парсер.ТипРазделитель Тогда - Если Разделитель = ТекущийТокен.Значение Тогда - СледующийТокен = Парсер.Следующий(); - Если СледующийТокен.Тип = Парсер.ТипКонецТекста Тогда - ВызватьИсключение СтрШаблон("Неожиданный конец строки версии, нет данных после разделителя <%1>", Разделитель); - КонецЕсли; - Возврат; - КонецЕсли; - КонецЕсли; + Если ТекущийТокен.Тип = Парсер.ТипРазделитель + И Разделитель = ТекущийТокен.Значение Тогда + СледующийТокен = Парсер.Следующий(); + Если СледующийТокен.Тип = Парсер.ТипКонецТекста Тогда + ВызватьИсключение СтрШаблон("Неожиданный конец строки версии, нет данных после разделителя <%1>", Разделитель); + КонецЕсли; + Возврат; + КонецЕсли;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/testing.yml
(1 hunks)src/core/Классы/Версия.os
(8 hunks)tests/preReleasesTests.os
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/testing.yml
- tests/preReleasesTests.os
🔇 Additional comments (13)
src/core/Классы/Версия.os (13)
11-11
: Корректное переименование поля и инициализация массиваПереименование поля
НомерБилда
наМетаданныеСборки
улучшает соответствие терминологии semver. Корректная инициализация массива в конструкторе обеспечивает стабильность работы класса.Also applies to: 20-20
51-58
: Корректная реализация строкового представления согласно спецификации semverДобавленная логика формирования строки соответствует спецификации семантического версионирования: пререлизные версии добавляются после дефиса, метаданные сборки после знака плюс. Корректно используется разделитель "." между идентификаторами.
76-76
: Улучшен синтаксис тернарного оператораИспользование более краткой записи тернарного оператора делает код более читаемым.
98-98
: Исправлена проверка на количество элементов в массивеУточнение условия для правильной проверки наличия элементов в массиве пререлизных версий.
107-107
: Выделение логики сравнения пререлизов в отдельную функциюВынесение сложной логики сравнения пререлизных версий в отдельную функцию повышает читаемость и поддерживаемость кода. Это следует принципу единственной ответственности.
171-201
: Корректная реализация сравнения пререлизных версийРеализация соответствует спецификации semver:
- Больший набор пререлизных идентификаторов имеет больший приоритет при равенстве сравниваемых идентификаторов
- Числовые идентификаторы имеют меньший приоритет, чем текстовые
- Сравнение выполняется последовательно по элементам массива
Функция корректно обрабатывает все случаи сравнения, включая массивы разной длины.
212-226
: Улучшена логика парсинга с использованием токенизацииНовый подход с использованием токенизатора значительно улучшает обработку строк версий. Использование объекта
ПарсерВерсии
для разбора строки на токены позволяет более точно обрабатывать различные части версии и корректно обрабатывать ошибки.Обработка ошибок через блок
Попытка/Исключение
повышает надежность кода.
249-264
: Корректное чтение числовых компонентов версииМетод
ПрочитатьЧислоДоРазделителя
корректно обрабатывает числовые токены и ошибки парсинга. В случае получения токена неожидаемого типа, выбрасывается исключение с понятным описанием.
266-279
: Метод обработки хвостовых массивовРеализация метода
ПрочитатьХвостовыеМассивы
корректно проверяет наличие хвостовых частей версии (пререлиза и метаданных сборки) и делегирует их обработку соответствующим методам.
281-286
: Методы чтения пререлиза и метаданных сборкиМетоды
ПрочитатьПреРелиз
иПрочитатьМетаданныеСборки
корректно обрабатывают соответствующие разделы строки версии. Логика чтения у них очень похожа.Можно рассмотреть возможность объединения этих методов в один с параметризацией, но это не критично.
Also applies to: 288-293
295-307
: Метод сборки массива из компонентовМетод
СобратьМассивИзКомпонентов
корректно собирает массив компонентов до встречи указанного разделителя или конца текста. Реализация универсальна для обоих типов хвостовых массивов, что является хорошей практикой.
309-325
: Метод чтения компонентаМетод
ПрочитатьКомпонент
корректно обрабатывает числовые и текстовые компоненты. Для текстовых компонентов правильно реализована поддержка дефисов внутри компонента, что соответствует спецификации semver.
327-334
: Метод подготовки строки версииМетод
ПодготовитьКЧтению
корректно удаляет префикс "v" из строки версии, что делает обработку более гибкой.
@@ -95,7 +95,7 @@ | |||
КонецЕсли; | |||
|
|||
Если ПреРелиз.Количество() = 0 | |||
И ВходящаяВерсия.ПреРелиз.Количество() > 1 Тогда | |||
И ВходящаяВерсия.ПреРелиз.Количество() > 0 Тогда |
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.
Предыдущим коммитом
Реализована поддержка пререлизов и метаданных сборок по semver
closes #2
Нарушена обратная совместимость при которой все версии с пререлизами, но ошибками - считались валидными, но без пререлизов.
Версия нужна для бэкенда сайта oscript.io для корректной сортировки версий
Summary by CodeRabbit