-
Notifications
You must be signed in to change notification settings - Fork 33
Обновление jdk и зависимостей #155
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
base: master
Are you sure you want to change the base?
Conversation
theshadowco
commented
Dec 15, 2025
- переезд на kts формат gradle
- новый градл 8.14
- jdk 17
- обновлены зависимости до актуальных версий
2. новый градл 8.14 3. jdk 17
2. внесены правки
WalkthroughМиграция системы сборки с Gradle build.gradle на Kotlin DSL (build.gradle.kts), обновление Gradle wrapper с версии 5.2.1 на 8.14.3, изменение года авторского права на 2025 в множестве Java файлов, незначительный рефакторинг кода (модернизация синтаксиса, добавление новых методов экспорта покрытия в Utils.java). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Области, требующие особого внимания:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. 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.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/main/java/com/clouds42/LinesToCoverage.java (1)
126-132: Смешение типов индексов вgetRuleIndex.Метод
getRuleIndexвозвращает разные по семантике значения: дляTerminalNode— тип токена (symbol.getType()), а дляParserRuleContext— индекс правила (getRuleIndex()). В текущем использовании вgetRootParentэто безопасно, так какparentвсегда являетсяParserRuleContext, но для общего использования это может вызвать путаницу.Рекомендую добавить комментарий или переименовать метод для ясности.
- private static int getRuleIndex(ParseTree node) { + /** + * Returns rule index for ParserRuleContext or token type for TerminalNode. + * Note: semantics differ based on node type. + */ + private static int getRuleIndex(ParseTree node) {build.gradle.kts (2)
12-13: Обработка отсутствующего свойства EDT_LOCATION.Если свойство
EDT_LOCATIONне задано вgradle.properties, произойдёт исключение с неинформативным сообщением. Рекомендую добавить явную проверку.-val edtLocation = file(project.properties["EDT_LOCATION"] as String) -println("EDT location: $edtLocation") +val edtLocation = project.findProperty("EDT_LOCATION")?.toString() + ?: error("EDT_LOCATION property is not set in gradle.properties") +logger.lifecycle("EDT location: {}", file(edtLocation))
166-173: Улучшите обработку токена GitHub.Если токен не задан, используется пустая строка, что может привести к тихим ошибкам при попытке релиза.
githubRelease { - token(project.findProperty("github.token")?.toString() ?: "") + token(project.findProperty("github.token")?.toString() + ?: System.getenv("GITHUB_TOKEN") + ?: "") owner = "proDOOMman" repo = "Coverage41C"Также рассмотрите возможность логирования предупреждения, если токен не задан.
src/main/java/com/clouds42/Utils.java (1)
666-686: Рекомендуется использовать ProcessBuilder.
Runtime.exec(String)работает, но рекомендуется использоватьProcessBuilderдля лучшей обработки команд и аргументов, а также для совместимости с будущими версиями JDK.private static boolean isProcessIdRunning(String pid, String command) { logger.debug("Command [{}]", command); try { - Runtime rt = Runtime.getRuntime(); - Process pr = rt.exec(command); + ProcessBuilder pb = new ProcessBuilder(command.split("\\s+")); + pb.redirectErrorStream(true); + Process pr = pb.start(); InputStreamReader isReader = new InputStreamReader(pr.getInputStream());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar,!**/*.jarsrc/test/resources/coverage/configuration.xmlis excluded by!**/*.xmlsrc/test/resources/coverage/edt.xmlis excluded by!**/*.xml
📒 Files selected for processing (37)
build.gradle(0 hunks)build.gradle.kts(1 hunks)gradle.properties(1 hunks)gradle/wrapper/gradle-wrapper.properties(1 hunks)gradlew(2 hunks)gradlew.bat(1 hunks)settings.gradle(0 hunks)settings.gradle.kts(1 hunks)src/main/java/com/clouds42/AbstractDebugClient.java(1 hunks)src/main/java/com/clouds42/CommandLineOptions/ConnectionOptions.java(1 hunks)src/main/java/com/clouds42/CommandLineOptions/ConvertOptions.java(1 hunks)src/main/java/com/clouds42/CommandLineOptions/DebuggerOptions.java(2 hunks)src/main/java/com/clouds42/CommandLineOptions/FilterOptions.java(1 hunks)src/main/java/com/clouds42/CommandLineOptions/LoggingOptions.java(2 hunks)src/main/java/com/clouds42/CommandLineOptions/MetadataOptions.java(1 hunks)src/main/java/com/clouds42/CommandLineOptions/OutputOptions.java(1 hunks)src/main/java/com/clouds42/Commands/ConvertCommand.java(1 hunks)src/main/java/com/clouds42/Commands/CoverServer.java(1 hunks)src/main/java/com/clouds42/Commands/CoverageCommand.java(2 hunks)src/main/java/com/clouds42/Commands/SendCheckMessageCommand.java(1 hunks)src/main/java/com/clouds42/Commands/SendCleanMessageCommand.java(1 hunks)src/main/java/com/clouds42/Commands/SendDumpMessageCommand.java(1 hunks)src/main/java/com/clouds42/Commands/SendMessageCommand.java(1 hunks)src/main/java/com/clouds42/Commands/SendStatsMessageCommand.java(1 hunks)src/main/java/com/clouds42/Commands/SendStopMessageCommand.java(1 hunks)src/main/java/com/clouds42/Coverage41C.java(1 hunks)src/main/java/com/clouds42/DebugClient.java(1 hunks)src/main/java/com/clouds42/GlobalCallsFilter.java(1 hunks)src/main/java/com/clouds42/LinesToCoverage.java(2 hunks)src/main/java/com/clouds42/MyRuntimeDebugModelXmlSerializer.java(1 hunks)src/main/java/com/clouds42/PipeMessages.java(1 hunks)src/main/java/com/clouds42/Utils.java(4 hunks)src/test/java/com/clouds42/ConvertTest.java(1 hunks)src/test/java/com/clouds42/CoverageTest.java(3 hunks)src/test/java/com/clouds42/LinesToCoverageTest.java(1 hunks)src/test/java/com/clouds42/TestUtils.java(1 hunks)src/test/java/com/clouds42/UtilsTest.java(1 hunks)
💤 Files with no reviewable changes (2)
- settings.gradle
- build.gradle
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/com/clouds42/LinesToCoverage.java (1)
src/main/java/com/clouds42/GlobalCallsFilter.java (1)
GlobalCallsFilter(30-172)
src/main/java/com/clouds42/Utils.java (1)
src/main/java/com/clouds42/LinesToCoverage.java (1)
LinesToCoverage(33-133)
🔇 Additional comments (41)
src/main/java/com/clouds42/Commands/SendMessageCommand.java (1)
4-4: Обновление года авторского права выглядит корректно.Обновление года в заголовке лицензии до 2025 соответствует текущему году и является стандартной практикой сопровождения проекта.
src/test/java/com/clouds42/ConvertTest.java (1)
4-4: Обновление года авторского права корректно.Изменение года в заголовке лицензии согласуется с остальными файлами проекта.
src/main/java/com/clouds42/AbstractDebugClient.java (1)
4-4: Обновление года авторского права корректно.Заголовок лицензии обновлён до 2025 года в соответствии с изменениями в других файлах проекта.
src/main/java/com/clouds42/Commands/ConvertCommand.java (1)
4-4: Обновление года авторского права корректно.Изменение заголовка лицензии соответствует стандартной практике обновления года авторского права.
src/test/java/com/clouds42/UtilsTest.java (1)
4-4: Обновление года авторского права корректно.Заголовок лицензии обновлён в рамках общего обновления проекта. Тестовая логика не затронута.
src/main/java/com/clouds42/Commands/SendStatsMessageCommand.java (1)
4-4: LGTM!Обновление года авторского права выглядит корректно.
src/main/java/com/clouds42/Commands/SendDumpMessageCommand.java (1)
4-4: LGTM!Обновление года авторского права выглядит корректно.
src/main/java/com/clouds42/Commands/SendStopMessageCommand.java (1)
4-4: LGTM!Обновление года авторского права выглядит корректно.
src/main/java/com/clouds42/Commands/CoverServer.java (1)
4-4: LGTM!Обновление года авторского права выглядит корректно.
src/main/java/com/clouds42/MyRuntimeDebugModelXmlSerializer.java (1)
4-4: LGTM!Обновление года авторского права выглядит корректно.
src/main/java/com/clouds42/CommandLineOptions/ConvertOptions.java (1)
4-4: LGTM!Обновление года авторского права выглядит корректно.
src/main/java/com/clouds42/Commands/SendCheckMessageCommand.java (1)
4-4: Пропуск замечания: обновление года в лицензииОбновление года авторского права - это административное изменение без влияния на функциональность.
src/main/java/com/clouds42/DebugClient.java (1)
4-4: Пропуск замечания: обновление года в лицензииОбновление года авторского права - это административное изменение без влияния на функциональность.
src/main/java/com/clouds42/Coverage41C.java (2)
4-4: Пропуск замечания: обновление года в лицензииОбновление года авторского права - это административное изменение без влияния на функциональность.
32-32: Проверка: BuildConfig.APP_VERSION должен быть сгенерирован при сборкеНа строке 32 используется
com.clouds42.BuildConfig.APP_VERSION. Убедитесь, что в новомbuild.gradle.ktsправильно настроена генерация классаBuildConfigс версией приложения.Проверьте, что
build.gradle.ktsсодержит конфигурацию для созданияBuildConfigкласса с переменнойAPP_VERSION.settings.gradle.kts (1)
1-1: Конфигурация корректнаИмя корневого проекта правильно установлено в Kotlin DSL синтаксисе.
gradle.properties (1)
1-2: JVM параметры сборки выглядят приемлемымиНастройки параллельной сборки и JVM памяти (1GB heap, 512MB metaspace) разумны для размера проекта.
src/main/java/com/clouds42/GlobalCallsFilter.java (1)
4-4: Пропуск замечания: обновление года в лицензииОбновление года авторского права - это административное изменение без влияния на функциональность.
src/main/java/com/clouds42/PipeMessages.java (1)
4-4: Пропуск замечания: обновление года в лицензииОбновление года авторского права - это административное изменение без влияния на функциональность.
src/test/java/com/clouds42/TestUtils.java (1)
4-4: Пропуск замечания: обновление года в лицензииОбновление года авторского права - это административное изменение без влияния на функциональность.
src/main/java/com/clouds42/Commands/SendCleanMessageCommand.java (1)
4-4: LGTM!Обновление года в заголовке авторских прав корректно.
src/main/java/com/clouds42/CommandLineOptions/LoggingOptions.java (1)
37-39: LGTM!Упрощение возврата значения корректно. Поскольку picocli устанавливает
defaultValue = "false", полеverboseвсегда будет инициализировано.src/main/java/com/clouds42/CommandLineOptions/DebuggerOptions.java (1)
94-102: LGTM!Использование diamond-оператора (
<>) — это идиоматичный подход для Java 7+ и улучшает читаемость кода без изменения поведения.src/main/java/com/clouds42/Commands/CoverageCommand.java (1)
260-262: LGTM!Использование
ifPresent()вместо явной проверкиisPresent()иget()— это более идиоматичный подход для работы сOptionalв современном Java.src/main/java/com/clouds42/CommandLineOptions/OutputOptions.java (1)
4-4: LGTM!Обновление года авторских прав корректно.
gradlew.bat (1)
1-94: LGTM!Стандартный скрипт Gradle wrapper для версии 8.x. Основные улучшения:
- Добавлен заголовок лицензии Apache 2.0
- Улучшена обработка ошибок с выводом в stderr
- Упрощён CLASSPATH и использован
-jarвместо-classpath- Добавлена валидация JAVA_HOME
gradle/wrapper/gradle-wrapper.properties (1)
3-5: Обновление Gradle к актуальной версии.Upgrade от Gradle 5.2.1 до 8.14.3 соответствует целям PR. Это последняя стабильная версия в ветке 8.14.x (релиз от 4 июля 2025). Добавление
validateDistributionUrl=trueулучшает безопасность проверкой SHA256 контрольной суммы. Смена с дистрибутива-allна-binсокращает размер загрузки, но убедитесь, что исходные коды Gradle не требуются для разработки.Необходимо проверить совместимость Gradle 8.14.3 со всеми используемыми плагинами перед слиянием.
gradlew (1)
1-251: Стандартный скрипт Gradle Wrapper.Это стандартный сгенерированный скрипт
gradlewот Gradle 8.x. Скрипт соответствует POSIX-стандартам и корректно обрабатывает различные операционные системы (Cygwin, MSYS, Darwin, NonStop). Изменения соответствуют обновлению Gradle wrapper до версии 8.14.src/main/java/com/clouds42/LinesToCoverage.java (3)
4-4: Обновление года авторского права.Корректное обновление года авторского права на 2025.
35-62: Миграция на ParserRuleContext выполнена корректно.Изменение типа с
BSLParserRuleContextнаParserRuleContextсоответствует обновлению зависимости bsl-parser. Все контексты, перечисленные в Set, наследуются отParserRuleContext, поэтому изменение обратно совместимо.
64-102: Логика извлечения строк корректна.Методы
getLines,mustCoveredиgetLineправильно обрабатывают новую иерархию типов. Особая обработкаMethodCallContextчерезgetRootParentдля получения корректного номера строки при цепочке вызовов — разумное решение.src/test/java/com/clouds42/LinesToCoverageTest.java (1)
1-193: Обновление года авторского права в тестовом файле.Изменение только в заголовке (год авторского права). Тестовые методы остались без изменений и обеспечивают хорошее покрытие различных сценариев: простые случаи, ошибки парсинга, условия, циклы, фильтрация и другие.
build.gradle.kts (3)
109-113: Нестандартная модификация classpath.Добавление
configurations.compileClasspath.get()в задачахJavaCompile,testиrun— нестандартный подход. Обычно Gradle управляет classpath автоматически. Возможно, это связано с использованиемflatDirдля EDT зависимостей. Убедитесь, что это необходимо и не приводит к дублированию путей в classpath.
1-173: Миграция на Kotlin DSL выполнена успешно.В целом, миграция с Groovy на Kotlin DSL build.gradle.kts выполнена корректно. Настроены все необходимые плагины, зависимости и задачи. Java 17 compatibility установлена правильно.
83-84: Версия2.20.1для Jackson соответствует последней стабильной версии (состояние на декабрь 2025). Данная версия актуальна и не требует обновления.Likely an incorrect or invalid review comment.
src/main/java/com/clouds42/Utils.java (3)
4-4: Обновление года авторского права.Корректное обновление года авторского права на 2025.
95-120: Использование switch expressions.Хорошее применение switch expressions (Java 14+), что соответствует миграции на JDK 17. Логика определения UUID модуля выглядит корректной.
85-687: Общая оценка Utils.java.Файл содержит обширную функциональность для работы с покрытием кода. Основные методы (
readMetadata,dumpCoverageFile, экспорт в различные форматы) реализованы корректно. Обратите внимание на указанные выше проблемы с управлением ресурсами.src/main/java/com/clouds42/CommandLineOptions/ConnectionOptions.java (1)
4-4: ✓ Обновление года авторского права согласовано.Обновление года авторского права с 2020-2024 на 2020-2025 выполнено корректно. Остальной код без изменений и выглядит хорошо.
src/main/java/com/clouds42/CommandLineOptions/MetadataOptions.java (1)
4-4: ✓ Обновление года авторского права согласовано.Год авторского права корректно обновлён с 2020-2024 на 2020-2025. Логика кода не изменена.
src/main/java/com/clouds42/CommandLineOptions/FilterOptions.java (1)
4-4: ✓ Обновление года авторского права согласовано.Год авторского права обновлён корректно на 2020-2025. Никаких функциональных изменений.
| org.gradle.parallel=true | ||
| org.gradle.jvmargs=-Xmx1024m -XX:MaxMetaspaceSize=512m | ||
|
|
||
| EDT_LOCATION=/opt/1C/1CE/components/1c-edt-2024.2.6+7-x86_64/plugins |
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.
🧩 Analysis chain
🏁 Script executed:
find . -name "build.gradle.kts" -o -name "build.gradle" | head -20Repository: 1c-syntax/Coverage41C
Length of output: 83
🏁 Script executed:
rg "EDT_LOCATION" --type gradle --type kotlin -B 3 -A 3Repository: 1c-syntax/Coverage41C
Length of output: 354
🏁 Script executed:
cat -n .gitignore | grep -E "(gradle\.properties|local\.properties)" || echo "Не найдено совпадений в .gitignore"Repository: 1c-syntax/Coverage41C
Length of output: 99
🏁 Script executed:
rg "EDT_LOCATION" --type properties -B 2 -A 2Repository: 1c-syntax/Coverage41C
Length of output: 99
🏁 Script executed:
git ls-files | grep gradle.propertiesRepository: 1c-syntax/Coverage41C
Length of output: 82
🏁 Script executed:
cat -n build.gradle.kts | head -50Repository: 1c-syntax/Coverage41C
Length of output: 1435
🏁 Script executed:
rg "edtLocation" --type kotlin -B 2 -A 5Repository: 1c-syntax/Coverage41C
Length of output: 911
🏁 Script executed:
cat -n gradle.propertiesRepository: 1c-syntax/Coverage41C
Length of output: 245
EDT_LOCATION жестко закодирован и создаст проблемы при локальной разработке
Абсолютный путь /opt/1C/1CE/components/1c-edt-2024.2.6+7-x86_64/plugins используется в build.gradle.kts как:
- Репозиторий для загрузки артефактов (строка 20)
- Источник JAR файлов с отладочной информацией (строка 39-41)
Проблемы:
- Linux-специфичный путь несовместим с Windows
- Зависит от точной версии EDK (2024.2.6+7)
- На разных машинах EDK может быть установлен в другое место
gradle.propertiesкоммичена в репозиторий, поэтому разработчикам придётся либо менять файл локально, либо использовать переменные окружения (не задокументировано)- Сломает сборку в CI/CD и у большинства разработчиков
Рекомендуется:
- Переместить
gradle.propertiesв.gitignoreи создатьgradle.properties.sampleкак шаблон - Задокументировать способ переопределения:
export ORG_GRADLE_PROJECT_EDT_LOCATION=/path/to/edtилиgradle -PEDT_LOCATION=/path/to/edt - Добавить обработку ошибки в
build.gradle.ktsс понятным сообщением если EDT_LOCATION не задан
🤖 Prompt for AI Agents
В файле gradle.properties около строки 4 строка EDT_LOCATION жестко прописывает
Linux-специфичный абсолютный путь; это ломает сборку на других машинах/Windows и
привязывает к конкретной версии. Уберите коммит этого файла (перенесите
gradle.properties в .gitignore), добавьте gradle.properties.sample с ключом
EDT_LOCATION для шаблона, задокументируйте в README как переопределять значение
(export ORG_GRADLE_PROJECT_EDT_LOCATION=/path/to/edt или gradle
-PEDT_LOCATION=/path/to/edt) и измените build.gradle.kts так, чтобы оно
проверяло наличие свойства/переменной и выдавало понятную ошибку с инструкцией
как задать EDT_LOCATION (или поддерживало fallback/OS-agnostic поиск), вместо
использования жестко закодированного пути.
| @Disabled | ||
| class CoverageTest { |
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.
Отключение тестов требует пояснения.
Аннотация @Disabled отключает весь тестовый класс без указания причины. Это снижает покрытие тестами и может скрыть регрессии, вызванные миграцией на JDK 17 и Gradle 8.14.
Рекомендуется:
- Добавить причину в аннотацию:
@Disabled("причина"). - Создать задачу (issue) для восстановления тестов.
Хотите, чтобы я создал issue для отслеживания восстановления этих тестов?
🤖 Prompt for AI Agents
In src/test/java/com/clouds42/CoverageTest.java around lines 43-44 the test
class is globally disabled with @Disabled but no reason is provided; update the
annotation to include a concise justification like @Disabled("explain reason
e.g. failing on JDK17/Gradle8.14 - tracked by ISSUE-123"), and create a project
issue (e.g. ISSUE-123) that documents the failure, steps to reproduce, and an
owner to re-enable the tests; reference that issue key in the annotation so
readers can find the tracking ticket.