build: Add boilerplate Maven project. #6
Merged
Conversation
This file contains 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
WalkthroughThe pull request introduces several updates across multiple files. A new Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (6)
src/test/java/com/yscope/logging/log4j2/BoilerplateTests.java (1)
7-12: Enhance test readability and maintainability
While the test is functional, consider these improvements to align with JUnit 5 best practices:
- Make the test method public
- Swap assertion parameters to follow the convention (actual, expected)
- Add test documentation and display name
Here's the suggested improvement:
package com.yscope.logging.log4j2;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.DisplayName;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class BoilerplateTests {
+ /**
+ * Verifies that the Boilerplate greeting returns the expected message.
+ */
@Test
- void testGreeting() {
- assertEquals(Boilerplate.getGreeting(), "Hello, world!");
+ @DisplayName("Should return hello world greeting")
+ public void testGreeting() {
+ assertEquals("Hello, world!", Boilerplate.getGreeting());
}
}README.md (4)
13-16: Consider enhancing the Requirements section
The requirements are clear but could benefit from additional context. Consider adding brief explanations for the version requirements and formatting the list more formally.
## Requirements
-* Java 11
-* Maven 3.8 or newer
+* Java Development Kit (JDK) 11 or higher
+* Apache Maven 3.8 or newer18-27: Add more context to build instructions
While the commands are correct, consider adding information about:
- Expected build outcomes
- Location of generated artifacts
- Common build errors and solutions
## Building
Build and test:
```shell
mvn package+The build artifacts will be generated in the target directory.
Build without testing:
mvn package -DskipTests+Note: Skipping tests is not recommended for production builds.
---
`29-32`: **Expand the Testing section**
The test command is correct but could benefit from additional information about:
- Test report locations
- How to view test coverage
- Running specific test categories
```diff
## Testing
```shell
mvn test
+Test reports can be found in the target/surefire-reports directory.
+
+To run a specific test class:
+shell +mvn test -Dtest=TestClassName +
---
`13-33`: **LGTM! Consider adding project architecture overview**
The new sections provide clear and accurate information for contributors. The structure is logical, and the commands are correct. Consider adding a high-level architecture overview section to help new contributors understand the project structure better.
</details>
<details>
<summary>pom.xml (1)</summary>
`12-15`: **Consider enhancing the project description.**
The current description is quite brief. Consider adding more details about:
- The specific types of appenders provided
- Key features and benefits
- Target use cases
</details>
</details>
<details>
<summary>📜 Review details</summary>
**Configuration used: CodeRabbit UI**
**Review profile: CHILL**
<details>
<summary>📥 Commits</summary>
Files that changed from the base of the PR and between f7a216cabdb9b0769681f917a26991d51f57bc4b and 83b5744e658fbf6d726ffb46c1212d7ead9312ea.
</details>
<details>
<summary>📒 Files selected for processing (4)</summary>
* README.md (1 hunks)
* pom.xml (1 hunks)
* src/main/java/com/yscope/logging/log4j2/Boilerplate.java (1 hunks)
* src/test/java/com/yscope/logging/log4j2/BoilerplateTests.java (1 hunks)
</details>
<details>
<summary>✅ Files skipped from review due to trivial changes (1)</summary>
* src/main/java/com/yscope/logging/log4j2/Boilerplate.java
</details>
<details>
<summary>🔇 Additional comments (2)</summary>
<details>
<summary>src/test/java/com/yscope/logging/log4j2/BoilerplateTests.java (1)</summary>
`1-6`: **LGTM! Package structure and imports are well-organized.**
The package naming and import statements follow Maven conventions and JUnit 5 best practices.
</details>
<details>
<summary>pom.xml (1)</summary>
`1-270`: **Overall structure looks good!**
The POM file provides a solid foundation for the Log4j 2 Appenders project. It includes essential build configurations, proper plugin management, and release profiles. The suggested improvements will enhance the project's maintainability and reliability.
</details>
</details>
</details>
<!-- This is an auto-generated comment by CodeRabbit for review status -->
pom.xml
Outdated
Comment on lines
39
to
41
| <properties> | ||
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| </properties> |
There was a problem hiding this comment.
Add required Log4j 2 dependencies and externalize versions.
- Essential dependencies are missing:
org.apache.logging.log4j:log4j-core(required for appender development)org.apache.logging.log4j:log4j-api(required for logging interfaces)
- Consider externalizing dependency versions to properties for easier maintenance.
Apply this diff:
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <log4j.version>2.23.1</log4j.version>
+ <junit.version>5.11.3</junit.version>
</properties>
<dependencies>
+ <!-- Log4j 2 Core Dependencies -->
+ <dependency>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-core</artifactId>
+ <version>${log4j.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-api</artifactId>
+ <version>${log4j.version}</version>
+ </dependency>
+
<!-- Dependencies for testing -->
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
- <version>5.11.3</version>
+ <version>${junit.version}</version>
<scope>test</scope>
</dependency>
</dependencies>Also applies to: 43-51
pom.xml
Outdated
Comment on lines
53
to
208
| <plugin> | ||
| <artifactId>maven-surefire-plugin</artifactId> | ||
| <version>3.5.1</version> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>org.codehaus.mojo</groupId> | ||
| <artifactId>build-helper-maven-plugin</artifactId> | ||
| <version>3.6.0</version> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>org.codehaus.mojo</groupId> | ||
| <artifactId>exec-maven-plugin</artifactId> | ||
| <version>3.5.0</version> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>org.codehaus.mojo</groupId> | ||
| <artifactId>versions-maven-plugin</artifactId> | ||
| <version>2.17.1</version> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>org.sonatype.plugins</groupId> | ||
| <artifactId>nexus-staging-maven-plugin</artifactId> | ||
| <version>1.7.0</version> | ||
| </plugin> | ||
| </plugins> | ||
| </pluginManagement> | ||
| <plugins> | ||
| <!-- Maven processes plugins | ||
| a) in the order of the phases in the lifecycle, and | ||
| b) in order of appearance within each phase. | ||
| When configuring plugins below, we do our best to order and group by | ||
| lifecycle phases. --> | ||
|
|
||
| <!-- No phase --> | ||
| <plugin> | ||
| <groupId>org.codehaus.mojo</groupId> | ||
| <artifactId>versions-maven-plugin</artifactId> | ||
| <configuration> | ||
| <generateBackupPoms>false</generateBackupPoms> | ||
| </configuration> | ||
| </plugin> | ||
|
|
||
| <!-- phase:validate --> | ||
| <plugin> | ||
| <artifactId>maven-enforcer-plugin</artifactId> | ||
| <executions> | ||
| <execution> | ||
| <id>enforce-maven</id> | ||
| <goals> | ||
| <goal>enforce</goal> | ||
| </goals> | ||
| <configuration> | ||
| <rules> | ||
| <requireMavenVersion> | ||
| <version>3.8</version> | ||
| </requireMavenVersion> | ||
| </rules> | ||
| </configuration> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
|
|
||
| <!-- phase:package --> | ||
| <plugin> | ||
| <artifactId>maven-javadoc-plugin</artifactId> | ||
| <executions> | ||
| <execution> | ||
| <id>attach-javadocs</id> | ||
| <goals> | ||
| <goal>jar</goal> | ||
| </goals> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| <plugin> | ||
| <artifactId>maven-source-plugin</artifactId> | ||
| <executions> | ||
| <execution> | ||
| <id>attach-sources</id> | ||
| <goals> | ||
| <goal>jar-no-fork</goal> | ||
| </goals> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
|
|
||
| <!-- phase:test-compile --> | ||
| <plugin> | ||
| <artifactId>maven-compiler-plugin</artifactId> | ||
| <configuration> | ||
| <release>11</release> | ||
| </configuration> | ||
| </plugin> | ||
|
|
||
| <!-- phase:test --> | ||
| <plugin> | ||
| <artifactId>maven-surefire-plugin</artifactId> | ||
| </plugin> | ||
| </plugins> | ||
| </build> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add JaCoCo for test coverage reporting.
Consider adding JaCoCo Maven plugin to generate test coverage reports, which is essential for maintaining code quality.
Add this configuration to the <plugins> section:
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<version>0.8.11</version>
<executions>
<execution>
<goals>
<goal>prepare-agent</goal>
</goals>
</execution>
<execution>
<id>report</id>
<phase>test</phase>
<goals>
<goal>report</goal>
</goals>
</execution>
</executions>
</plugin>
Comment on lines
+211
to
+220
| <profile> | ||
| <id>github_release</id> | ||
| <distributionManagement> | ||
| <repository> | ||
| <id>github</id> | ||
| <name>GitHub Packages</name> | ||
| <url>https://maven.pkg.github.com/${env.GITHUB_REPOSITORY}</url> | ||
| </repository> | ||
| </distributionManagement> | ||
| </profile> |
There was a problem hiding this comment.
Add fallback for GITHUB_REPOSITORY environment variable.
The GitHub release profile might fail if the environment variable is not set. Consider adding a property with a default value.
Add this to the <properties> section:
<github.repository>y-scope/log4j2-appenders</github.repository>Then modify the URL:
<repository>
<id>github</id>
<name>GitHub Packages</name>
- <url>https://maven.pkg.github.com/${env.GITHUB_REPOSITORY}</url>
+ <url>https://maven.pkg.github.com/${github.repository}</url>
</repository>
Comment on lines
+129
to
+132
| <groupId>org.sonatype.plugins</groupId> | ||
| <artifactId>nexus-staging-maven-plugin</artifactId> | ||
| <version>1.7.0</version> | ||
| </plugin> |
There was a problem hiding this comment.
Update nexus-staging-maven-plugin version.
The current version (1.7.0) is quite old. The latest version is 1.6.13 (note: version numbering is counterintuitive).
Apply this diff:
<plugin>
<groupId>org.sonatype.plugins</groupId>
<artifactId>nexus-staging-maven-plugin</artifactId>
- <version>1.7.0</version>
+ <version>1.6.13</version>
</plugin>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <groupId>org.sonatype.plugins</groupId> | |
| <artifactId>nexus-staging-maven-plugin</artifactId> | |
| <version>1.7.0</version> | |
| </plugin> | |
| <groupId>org.sonatype.plugins</groupId> | |
| <artifactId>nexus-staging-maven-plugin</artifactId> | |
| <version>1.6.13</version> | |
| </plugin> |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
pom.xml (1)
224-284: Add profile documentation in README.
The pom.xml defines two release profiles that should be documented in the README:
github_releasefor publishing to GitHub Packagesmaven_releasefor publishing to Maven Central
Add this section to README.md:
## Publishing
### GitHub Packages
To publish to GitHub Packages:
```bash
mvn deploy -P github_releaseMaven Central
To publish to Maven Central:
mvn deploy -P maven_releaseNote: Requires GPG key setup and Sonatype OSSRH credentials.
</details>
</details>
<details>
<summary>📜 Review details</summary>
**Configuration used: CodeRabbit UI**
**Review profile: CHILL**
<details>
<summary>📥 Commits</summary>
Files that changed from the base of the PR and between 83b5744e658fbf6d726ffb46c1212d7ead9312ea and 504d69205cb8ec08d6c4200edfe82ef2d24034de.
</details>
<details>
<summary>📒 Files selected for processing (4)</summary>
* .gitignore (1 hunks)
* README.md (1 hunks)
* pom.xml (1 hunks)
* src/test/java/com/yscope/logging/log4j2/BoilerplateTests.java (1 hunks)
</details>
<details>
<summary>✅ Files skipped from review due to trivial changes (1)</summary>
* .gitignore
</details>
<details>
<summary>🚧 Files skipped from review as they are similar to previous changes (2)</summary>
* README.md
* src/test/java/com/yscope/logging/log4j2/BoilerplateTests.java
</details>
<details>
<summary>🔇 Additional comments (1)</summary>
<details>
<summary>pom.xml (1)</summary>
`1-38`: **LGTM! Project metadata is well-structured.**
The project coordinates, licensing, and SCM information are properly configured.
</details>
</details>
</details>
<!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+39
to
+65
| <properties> | ||
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| <org.apache.logging.log4j.version>2.24.1</org.apache.logging.log4j.version> | ||
| <org.junit.jupiter.version>5.11.3</org.junit.jupiter.version> | ||
| </properties> | ||
|
|
||
| <dependencies> | ||
| <!-- Provided dependencies --> | ||
| <dependency> | ||
| <groupId>org.apache.logging.log4j</groupId> | ||
| <artifactId>log4j-api</artifactId> | ||
| <version>${org.apache.logging.log4j.version}</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.apache.logging.log4j</groupId> | ||
| <artifactId>log4j-core</artifactId> | ||
| <version>${org.apache.logging.log4j.version}</version> | ||
| </dependency> | ||
|
|
||
| <!-- Dependencies for testing --> | ||
| <dependency> | ||
| <groupId>org.junit.jupiter</groupId> | ||
| <artifactId>junit-jupiter-engine</artifactId> | ||
| <version>${org.junit.jupiter.version}</version> | ||
| <scope>test</scope> | ||
| </dependency> | ||
| </dependencies> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding additional testing dependencies.
While JUnit Jupiter is included, consider adding these commonly used testing libraries:
org.assertj:assertj-corefor fluent assertionsorg.mockito:mockito-junit-jupiterfor mocking in tests
Add these dependencies:
<!-- Dependencies for testing -->
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<version>3.25.3</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-junit-jupiter</artifactId>
<version>5.11.0</version>
<scope>test</scope>
</dependency>
Comment on lines
+67
to
+222
| <plugin> | ||
| <artifactId>maven-surefire-plugin</artifactId> | ||
| <version>3.5.1</version> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>org.codehaus.mojo</groupId> | ||
| <artifactId>build-helper-maven-plugin</artifactId> | ||
| <version>3.6.0</version> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>org.codehaus.mojo</groupId> | ||
| <artifactId>exec-maven-plugin</artifactId> | ||
| <version>3.5.0</version> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>org.codehaus.mojo</groupId> | ||
| <artifactId>versions-maven-plugin</artifactId> | ||
| <version>2.17.1</version> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>org.sonatype.plugins</groupId> | ||
| <artifactId>nexus-staging-maven-plugin</artifactId> | ||
| <version>1.7.0</version> | ||
| </plugin> | ||
| </plugins> | ||
| </pluginManagement> | ||
| <plugins> | ||
| <!-- Maven processes plugins | ||
| a) in the order of the phases in the lifecycle, and | ||
| b) in order of appearance within each phase. | ||
| When configuring plugins below, we do our best to order and group by | ||
| lifecycle phases. --> | ||
|
|
||
| <!-- No phase --> | ||
| <plugin> | ||
| <groupId>org.codehaus.mojo</groupId> | ||
| <artifactId>versions-maven-plugin</artifactId> | ||
| <configuration> | ||
| <generateBackupPoms>false</generateBackupPoms> | ||
| </configuration> | ||
| </plugin> | ||
|
|
||
| <!-- phase:validate --> | ||
| <plugin> | ||
| <artifactId>maven-enforcer-plugin</artifactId> | ||
| <executions> | ||
| <execution> | ||
| <id>enforce-maven</id> | ||
| <goals> | ||
| <goal>enforce</goal> | ||
| </goals> | ||
| <configuration> | ||
| <rules> | ||
| <requireMavenVersion> | ||
| <version>3.8</version> | ||
| </requireMavenVersion> | ||
| </rules> | ||
| </configuration> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
|
|
||
| <!-- phase:package --> | ||
| <plugin> | ||
| <artifactId>maven-javadoc-plugin</artifactId> | ||
| <executions> | ||
| <execution> | ||
| <id>attach-javadocs</id> | ||
| <goals> | ||
| <goal>jar</goal> | ||
| </goals> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| <plugin> | ||
| <artifactId>maven-source-plugin</artifactId> | ||
| <executions> | ||
| <execution> | ||
| <id>attach-sources</id> | ||
| <goals> | ||
| <goal>jar-no-fork</goal> | ||
| </goals> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
|
|
||
| <!-- phase:test-compile --> | ||
| <plugin> | ||
| <artifactId>maven-compiler-plugin</artifactId> | ||
| <configuration> | ||
| <release>11</release> | ||
| </configuration> | ||
| </plugin> | ||
|
|
||
| <!-- phase:test --> | ||
| <plugin> | ||
| <artifactId>maven-surefire-plugin</artifactId> | ||
| </plugin> | ||
| </plugins> | ||
| </build> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add code quality plugins.
Consider adding these essential code quality plugins:
- Checkstyle for enforcing coding standards
- SpotBugs for finding potential bugs
Add to pluginManagement:
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
<version>3.3.1</version>
</plugin>
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<version>4.8.3.1</version>
</plugin>And to plugins section:
<!-- phase:verify -->
<plugin>
<artifactId>maven-checkstyle-plugin</artifactId>
<configuration>
<configLocation>google_checks.xml</configLocation>
</configuration>
<executions>
<execution>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<executions>
<execution>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>
jackluo923
approved these changes
Oct 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
This adds a boilerplate Maven project that contributors can build on top of. The
pom.xmlfile is the amalgamation of best practices from clp-ffi-java, log4j1-appenders, and new learning.Validation performed
Validated these commands succeeded:
mvn packagemvn package -DskipTestsmvn testSummary by CodeRabbit
New Features
Boilerplatewith a greeting method.Tests
BoilerplateTeststo verify the greeting functionality.Chores
.gitignoreto include IDE-related configurations.