Conversation
Uncommented test bundle modules in the root pom.xml to include them in the Tycho build. Removed Eclipse-specific .project files from the respective test bundle directories as they are not needed for Maven/Tycho builds.
Moved com.vogella.tycho.plugin1.tests, com.vogella.tycho.plugin1.tests.junit4, com.vogella.tycho.rcp.tests, and com.vogella.tycho.rcp.it.tests into a new 'tests' folder. Updated root pom.xml to point to the new locations.
Removed the 'com.vogella.tycho.fatjar' directory as it is no longer needed. The module was already implicitly removed from pom.xml during a previous refactor.
Moved the 'com.vogella.tycho.plugin1.test.junit4' module into the 'tests' directory and updated the root pom.xml to reflect its new location.
Renamed 'tests/com.vogella.tycho.plugin1.tests.junit4' to 'tests/com.vogella.tycho.plugin1.junit4.tests' and updated its Bundle-SymbolicName to match. This ensures Tycho pomless build recognizes it as a test bundle. Also removed the empty 'tests/com.vogella.tycho.plugin1.test.junit4' directory and its reference in pom.xml.
Updated the Tycho version from 4.0.9 to 5.0.0 in both the root pom.xml and .mvn/extensions.xml.
Updated the relativePath in tests/com.vogella.tycho.rcp.it.tests/pom.xml to correctly point to the parent pom.xml after moving the module into the 'tests' directory.
Uncommented the JUnit 5 bundles in releng/target-platform/target-platform.target and specified version 5.7.0 for org.junit.jupiter.api to resolve missing dependency error.
Changed the Eclipse release repository from '2025-09' to '2022-12' in releng/target-platform/target-platform.target to resolve the 'org.eclipse.platform.sdk' not found error, as '2025-09' is likely a future or unavailable release.
Moved JUnit 5 bundles (org.junit.jupiter.api, org.junit.jupiter.engine, etc.) to the Eclipse Orbit repository in releng/target-platform/target-platform.target. Set org.junit.jupiter.api and org.junit.jupiter.engine versions to 5.7.0. Removed the now redundant 2022-12 location block for these JUnit units.
Moved JUnit 5 bundles from Orbit to a new location block in releng/target-platform/target-platform.target, pointing to 'https://download.eclipse.org/releases/latest' to improve resolution.
Changed the repository for JUnit 5 bundles in releng/target-platform/target-platform.target to 'https://download.eclipse.org/releases/2020-06' to improve resolution.
Updated releng/target-platform/target-platform.target: - Removed Orbit and old P2 repository locations. - Updated Eclipse release repository to 2025-12. - Removed version='0.0.0' from units. - Added JUnit 5 (Jupiter/Platform) and JUnit 4 as Maven dependencies to resolve resolution issues.
Reverted the Eclipse platform repository for SDK to '2022-12' to resolve 'org.eclipse.platform.sdk' not found errors. Added ch.qos.logback.classic, ch.qos.logback.core, and org.slf4j-api as Maven dependencies to the target platform.
Removed direct plugin dependencies for org.slf4j.api, ch.qos.logback.core, ch.qos.logback.slf4j, and ch.qos.logback.classic from features/com.vogella.tycho.feature/feature.xml. These are now expected to be resolved via Maven dependencies in the target platform.
Use latest Tycho version Update GH build actions Include Junit test into run Update to Java 21 BREE Update Libaries for Unit testing
Summary of ChangesHello @vogella, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly modernizes the project's build environment and dependencies. It primarily focuses on upgrading the Java version to 21, updating the Eclipse Tycho build system and target platform to recent versions, and migrating to Jakarta EE specifications. Additionally, it streamlines the project structure by consolidating test modules and enhancing documentation, ensuring the project remains current and maintainable. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request provides a significant update to the project, migrating to Java 21, Tycho 5, and a much newer Eclipse target platform (2025-09). The move to JUnit 5 and the refactoring of test modules are also great improvements. I've found a few issues, mostly related to configuration and documentation consistency, that should be addressed. Specifically, the new project documentation in GEMINI.md contains an outdated version, and the new target platform definition has some maintainability issues with comments and inconsistent dependency versions for JUnit.
| <dependencies> | ||
| <dependency> | ||
| <groupId>ch.qos.logback</groupId> | ||
| <artifactId>logback-classic</artifactId> | ||
| <version>1.2.11</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>ch.qos.logback</groupId> | ||
| <artifactId>logback-core</artifactId> | ||
| <version>1.2.11</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.jupiter</groupId> | ||
| <artifactId>junit-jupiter-api</artifactId> | ||
| <version>5.13.4</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.jupiter</groupId> | ||
| <artifactId>junit-jupiter-engine</artifactId> | ||
| <version>5.13.4</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.jupiter</groupId> | ||
| <artifactId>junit-jupiter-params</artifactId> | ||
| <version>5.13.4</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.platform</groupId> | ||
| <artifactId>junit-platform-commons</artifactId> | ||
| <version>1.13.4</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.platform</groupId> | ||
| <artifactId>junit-platform-engine</artifactId> | ||
| <version>1.13.4</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <!-- https://mvnrepository.com/artifact/org.junit.platform/junit-platform-launcher --> | ||
| <dependency> | ||
| <groupId>org.junit.platform</groupId> | ||
| <artifactId>junit-platform-launcher</artifactId> | ||
| <version>1.14.1</version> | ||
| </dependency> | ||
| <!-- https://mvnrepository.com/artifact/org.junit.platform/junit-platform-suite-commons --> | ||
| <dependency> | ||
| <groupId>org.junit.platform</groupId> | ||
| <artifactId>junit-platform-suite-commons</artifactId> | ||
| <version>1.14.0</version> | ||
| <scope>runtime</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.platform</groupId> | ||
| <artifactId>junit-platform-suite-api</artifactId> | ||
| <version>1.13.4</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.platform</groupId> | ||
| <artifactId>junit-platform-suite-engine</artifactId> | ||
| <version>1.14.1</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.mockito</groupId> | ||
| <artifactId>mockito-core</artifactId> | ||
| <version>5.12.0</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.objenesis</groupId> | ||
| <artifactId>objenesis</artifactId> | ||
| <version>3.2</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.slf4j</groupId> | ||
| <artifactId>slf4j-api</artifactId> | ||
| <version>1.7.32</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| </dependencies> |
There was a problem hiding this comment.
There are multiple different versions used for JUnit 5 artifacts (5.13.4, 1.13.4, 1.14.0, 1.14.1). This can lead to classpath issues and unexpected behavior at runtime. It is highly recommended to use a consistent set of versions for all related JUnit 5 artifacts. Consider managing these versions with properties to ensure consistency and maintainability.
| ## Key Technologies | ||
| * **Java:** (Version 21 is used in CI, targeting Java 17+) | ||
| * **Maven:** Build tool (Version 3.9.8+ recommended) | ||
| * **Eclipse Tycho:** Maven extensions for building Eclipse artifacts (Version 4.0.9) |
There was a problem hiding this comment.
The documentation mentions Eclipse Tycho version 4.0.9, but this pull request updates it to 5.0.0. Please update the documentation to reflect the new version.
| * **Eclipse Tycho:** Maven extensions for building Eclipse artifacts (Version 4.0.9) | |
| * **Eclipse Tycho:** Maven extensions for building Eclipse artifacts (Version 5.0.0) |
| <version>1.13.4</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <!-- https://mvnrepository.com/artifact/org.junit.platform/junit-platform-launcher --> |
There was a problem hiding this comment.
This comment linking to mvnrepository.com adds noise to the configuration file and should be removed to improve readability. The dependency information is self-contained within the <dependency> tag that follows.
| <artifactId>junit-platform-launcher</artifactId> | ||
| <version>1.14.1</version> | ||
| </dependency> | ||
| <!-- https://mvnrepository.com/artifact/org.junit.platform/junit-platform-suite-commons --> |
There was a problem hiding this comment.
This comment linking to mvnrepository.com adds noise to the configuration file and should be removed to improve readability. The dependency information is self-contained within the <dependency> tag that follows.
There was a problem hiding this comment.
Code Review
This pull request updates project dependencies to newer versions, including Tycho 5.0.0 and Java 21, and restructures the test modules. While these are positive changes, there are several critical issues that need to be addressed. The target platform configuration points to a non-existent Eclipse release and uses dependency versions for JUnit that do not seem to exist, which will cause the build to fail. Furthermore, the process of moving test files into a new directory structure appears to have been done incorrectly, mixing files from different modules. The new documentation file also contains some outdated information. These issues must be resolved before this pull request can be merged.
| includeSource="true" type="InstallableUnit"> | ||
| <repository location="https://download.eclipse.org/releases/2022-12" /> | ||
| <location includeAllPlatforms="false" includeConfigurePhase="true" includeMode="planner" includeSource="true" type="InstallableUnit"> | ||
| <repository location="https://download.eclipse.org/releases/2025-09"/> |
There was a problem hiding this comment.
The repository location https://download.eclipse.org/releases/2025-09 points to an Eclipse release that is far in the future and the URL is likely not yet active. This will cause a build failure. Please use a URL for a valid and existing Eclipse release.
| <dependency> | ||
| <groupId>org.junit.jupiter</groupId> | ||
| <artifactId>junit-jupiter-api</artifactId> | ||
| <version>5.13.4</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.jupiter</groupId> | ||
| <artifactId>junit-jupiter-engine</artifactId> | ||
| <version>5.13.4</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.jupiter</groupId> | ||
| <artifactId>junit-jupiter-params</artifactId> | ||
| <version>5.13.4</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.platform</groupId> | ||
| <artifactId>junit-platform-commons</artifactId> | ||
| <version>1.13.4</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.platform</groupId> | ||
| <artifactId>junit-platform-engine</artifactId> | ||
| <version>1.13.4</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <!-- https://mvnrepository.com/artifact/org.junit.platform/junit-platform-launcher --> | ||
| <dependency> | ||
| <groupId>org.junit.platform</groupId> | ||
| <artifactId>junit-platform-launcher</artifactId> | ||
| <version>1.14.1</version> | ||
| </dependency> | ||
| <!-- https://mvnrepository.com/artifact/org.junit.platform/junit-platform-suite-commons --> | ||
| <dependency> | ||
| <groupId>org.junit.platform</groupId> | ||
| <artifactId>junit-platform-suite-commons</artifactId> | ||
| <version>1.14.0</version> | ||
| <scope>runtime</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.platform</groupId> | ||
| <artifactId>junit-platform-suite-api</artifactId> | ||
| <version>1.13.4</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.platform</groupId> | ||
| <artifactId>junit-platform-suite-engine</artifactId> | ||
| <version>1.14.1</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.mockito</groupId> | ||
| <artifactId>mockito-core</artifactId> | ||
| <version>5.12.0</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.objenesis</groupId> | ||
| <artifactId>objenesis</artifactId> | ||
| <version>3.2</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.slf4j</groupId> | ||
| <artifactId>slf4j-api</artifactId> | ||
| <version>1.7.32</version> | ||
| <type>jar</type> | ||
| </dependency> |
There was a problem hiding this comment.
The versions specified for org.junit.jupiter and org.junit.platform artifacts (e.g., 5.13.4, 1.13.4, 1.14.1) do not appear to exist in public Maven repositories. This will cause dependency resolution to fail. Please use correct and compatible versions for all JUnit 5 artifacts. It is also recommended to use consistent versions across all related JUnit artifacts, for example by using a BOM.
| <module>tests/com.vogella.tycho.plugin1.tests</module> | ||
| <module>tests/com.vogella.tycho.rcp.tests</module> | ||
| <module>tests/com.vogella.tycho.rcp.it.tests</module> |
There was a problem hiding this comment.
The restructuring of test modules into the tests/ directory is a good improvement. However, the file moves seem to have been performed incorrectly, resulting in configuration files from one module being moved to another (e.g., .classpath from rcp.tests moved to plugin1.tests). This will likely break the IDE project setup and the build. Please review and correct all file moves for the test modules.
| ## Key Technologies | ||
| * **Java:** (Version 21 is used in CI, targeting Java 17+) | ||
| * **Maven:** Build tool (Version 3.9.8+ recommended) | ||
| * **Eclipse Tycho:** Maven extensions for building Eclipse artifacts (Version 4.0.9) |
There was a problem hiding this comment.
The Tycho version has been updated to 5.0.0 in this pull request, but this documentation still refers to the old version 4.0.9. Please update this line to reflect the correct version.
| * **Eclipse Tycho:** Maven extensions for building Eclipse artifacts (Version 4.0.9) | |
| * **Eclipse Tycho:** Maven extensions for building Eclipse artifacts (Version 5.0.0) |
| * `releng/`: Release engineering artifacts. | ||
| * `com.vogella.tycho.product/`: Defines the product build. | ||
| * `target-platform/`: Contains the `.target` definition file used to resolve dependencies. | ||
| * `com.vogella.tycho.fatjar/`: Example of a fat jar build. |
There was a problem hiding this comment.
The com.vogella.tycho.fatjar module has been removed in this pull request, but it is still mentioned here. This line should be removed to keep the documentation accurate.
| <!-- https://mvnrepository.com/artifact/org.junit.platform/junit-platform-launcher --> | ||
| <dependency> | ||
| <groupId>org.junit.platform</groupId> | ||
| <artifactId>junit-platform-launcher</artifactId> | ||
| <version>1.14.1</version> | ||
| </dependency> | ||
| <!-- https://mvnrepository.com/artifact/org.junit.platform/junit-platform-suite-commons --> | ||
| <dependency> | ||
| <groupId>org.junit.platform</groupId> | ||
| <artifactId>junit-platform-suite-commons</artifactId> | ||
| <version>1.14.0</version> | ||
| <scope>runtime</scope> | ||
| </dependency> |
There was a problem hiding this comment.
The XML comments with links to mvnrepository.com add unnecessary clutter to this configuration file and should be removed. Additionally, the indentation within this block is inconsistent, which makes the file harder to read. Please remove the comments and fix the indentation.
1 participant
No description provided.