junie guidelines #2166
junie guidelines #2166
Conversation
WalkthroughA new documentation file was added introducing project-specific development guidelines for the Membrane API Gateway. It covers environment setup, repository structure, build/test commands, local running, conventions, code style, troubleshooting, release/packaging, and references. No code, API, or interface changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
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 |
|
This pull request needs "/ok-to-test" from an authorized committer. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.junie/guidelines.md (1)
120-121: Format URLs properly in Markdown.The bare URLs should be formatted as proper Markdown links for better presentation and accessibility.
Apply this diff to format the URLs properly:
-Contact/Upstream -- Upstream: https://github.com/membrane/api-gateway -- Issues: https://github.com/membrane/api-gateway/issues +Contact/Upstream +- Upstream: [https://github.com/membrane/api-gateway](https://github.com/membrane/api-gateway) +- Issues: [https://github.com/membrane/api-gateway/issues](https://github.com/membrane/api-gateway/issues)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.junie/guidelines.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-22T13:18:09.463Z
Learnt from: predic8
PR: membrane/api-gateway#2068
File: core/src/test/java/com/predic8/membrane/core/interceptor/balancer/ClusterBalancerTest.java:73-90
Timestamp: 2025-08-22T13:18:09.463Z
Learning: The Membrane API Gateway project has mixed Java version configurations in different modules, but the team uses Java 21 in practice. Modern Java 21 features like List.getFirst() are appropriate and should be used instead of older alternatives like List.get(0).
Applied to files:
.junie/guidelines.md
🪛 markdownlint-cli2 (0.18.1)
.junie/guidelines.md
120-120: Bare URL used
(MD034, no-bare-urls)
121-121: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (4)
.junie/guidelines.md (4)
1-122: Excellent comprehensive development guidelines document!This is a well-structured and thorough guidelines document that provides clear guidance for developers working on the Membrane API Gateway. The documentation covers all essential aspects from environment setup to testing, troubleshooting, and release processes.
10-10: Java 21 usage confirmed and aligned with project standards.Good to see explicit documentation of Java 21 as the target version, which aligns with the retrieved learnings indicating the team uses Java 21 features in practice.
70-89: Good practice: Temporary test verification approach.The approach of adding a temporary test to validate the workflow and then removing it to keep the repository clean is a good practice for documentation verification.
95-96: Valuable SpEL functions guidance.The specific guidance about SpEL functions requiring static methods with SpELExchangeEvaluationContext as the last parameter is valuable domain-specific knowledge that will help developers avoid common pitfalls.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
.junie/guidelines.md (5)
1-1: Use proper Markdown headings for scannability.Promote the title to an H1 for better structure and anchor generation.
-Membrane API Gateway — Project-specific Development Guidelines +# Membrane API Gateway — Project-specific Development Guidelines
5-5: Timestamp clarity and durability.Add timezone and the commit hash used for verification to avoid ambiguity.
-Last verified with: Java 21, Maven 3.9+, Windows dev environment. Date: 2025-09-22 09:55 local. +Last verified with: Java 21, Maven 3.9+, Windows dev environment. +Verified at: 2025-09-22T09:55-04:00 (commit: <SHA>). Update when environment changes.
37-39: Make path examples cross‑platform.Backslash-only paths can mislead Linux/macOS users. Provide both POSIX and Windows variants.
- - After packaging distribution: distribution\target contains router binaries and scripts. - - Configuration is under distribution\conf\proxies.xml (example provided). Edit/extend this file to add routes. + - After packaging distribution: distribution/target (Unix/macOS) or distribution\target (Windows) contains router binaries and scripts. + - Configuration is under distribution/conf/proxies.xml (Unix/macOS) or distribution\conf\proxies.xml (Windows). Edit/extend this file to add routes. @@ -- Windows path note: Always use backslashes for file paths when referencing resources; Maven parameters still use Java FQNs for -Dtest, not file paths. +- Path note: Use forward slashes on Unix/macOS (e.g., distribution/conf) and backslashes on Windows (e.g., distribution\conf). Maven parameters still use Java FQNs for -Dtest, not file paths. @@ - - Conf files under distribution\conf are used by RouterCLI and scripts. If you change schema in core, ensure router-conf.xsd copy step still functions and docs generation remains intact. + - Conf files under distribution/conf (Unix/macOS) or distribution\conf (Windows) are used by RouterCLI and scripts. If you change schema in core, ensure router-conf.xsd copy step still functions and docs generation remains intact. @@ -- Full release artifacts are assembled under distribution\target after mvn package at the root. WAR packaging is in war module for servlet containers. +- Full release artifacts are assembled under distribution/target (Unix/macOS) or distribution\target (Windows) after mvn package at the root. WAR packaging is in war module for servlet containers.Also applies to: 56-56, 97-99, 117-117
74-85: Fence the Java snippet for syntax highlighting and correct rendering.Without a fenced block, this may not render as intended.
- package com.predic8.membrane.core; - - import org.junit.jupiter.api.Test; - import static org.junit.jupiter.api.Assertions.*; - - public class SanityTest { - @Test - void sumsWork() { - int a = 2 + 2; - assertEquals(4, a, "Basic arithmetic should work"); - } - } +```java +package com.predic8.membrane.core; + +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; + +public class SanityTest { + @Test + void sumsWork() { + int a = 2 + 2; + assertEquals(4, a, "Basic arithmetic should work"); + } +} +```
120-121: Fix markdownlint MD034 (no-bare-urls).Convert bare URLs to links.
-- Upstream: https://github.com/membrane/api-gateway -- Issues: https://github.com/membrane/api-gateway/issues +- Upstream: <https://github.com/membrane/api-gateway> +- Issues: <https://github.com/membrane/api-gateway/issues>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
-
.junie/guidelines.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-22T13:18:09.463Z
Learnt from: predic8
PR: membrane/api-gateway#2068
File: core/src/test/java/com/predic8/membrane/core/interceptor/balancer/ClusterBalancerTest.java:73-90
Timestamp: 2025-08-22T13:18:09.463Z
Learning: The Membrane API Gateway project has mixed Java version configurations in different modules, but the team uses Java 21 in practice. Modern Java 21 features like List.getFirst() are appropriate and should be used instead of older alternatives like List.get(0).
Applied to files:
.junie/guidelines.md
🪛 markdownlint-cli2 (0.18.1)
.junie/guidelines.md
120-120: Bare URL used
(MD034, no-bare-urls)
121-121: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (3)
.junie/guidelines.md (3)
10-10: Java 21 alignment looks good.This matches team practice (use Java 21) and past guidance. No changes needed.
10-10: Confirmed: root POM uses javac.source / javac.target
root pom.xml defines <javac.source>21 and <javac.target>21 and the maven-compiler-plugin is configured to use ${javac.source}/${javac.target} (root pom.xml lines ~78–79 and plugin config ~368–369).
32-34: Verified — annotation processor XSD copy and CycloneDX SBOM are configured in core.Confirmed: core/pom.xml declares the org.membrane-soa:service-proxy-annot dependency and includes a cyclonedx-maven-plugin execution plus a maven-antrun-plugin execution that copies router-conf.xsd into ../docs (see core/pom.xml ~lines 45, 429, 459–471). annot/pom.xml defines artifactId service-proxy-annot.
| - mvn -q -pl core -DskipTests -DskipITs=false verify | ||
| - Failsafe runs during integration-test/verify phases. The default config in core binds it to look for IntegrationTestsWithInternet/IntegrationTestsWithoutInternet suites. | ||
| - Windows path note: Always use backslashes for file paths when referencing resources; Maven parameters still use Java FQNs for -Dtest, not file paths. |
There was a problem hiding this comment.
Command to run ITs only is misleading; -DskipTests also skips Failsafe.
Per Maven docs, skipTests is honored by Failsafe too; combining it with -DskipITs=false won’t run ITs. Recommend invoking Failsafe goals explicitly.
-- Run integration tests (if any configured in the module):
- - mvn -q -pl core -DskipTests -DskipITs=false verify
- - Failsafe runs during integration-test/verify phases. The default config in core binds it to look for IntegrationTestsWithInternet/IntegrationTestsWithoutInternet suites.
+- Run integration tests (if any configured in the module):
+ - Preferred (ITs only): mvn -q -pl core -am -DskipITs=false failsafe:integration-test failsafe:verify
+ - Run unit + IT together: mvn -q -pl core -am verify
+ - Note: -DskipTests is honored by Failsafe as well and skips ITs; do not combine it with -DskipITs=false.
+ - Failsafe runs during integration-test/verify phases. The default config in core binds it to look for IntegrationTestsWithInternet/IntegrationTestsWithoutInternet suites.References: Maven Failsafe “Skipping Tests” docs. (maven.apache.org)
📝 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.
| - mvn -q -pl core -DskipTests -DskipITs=false verify | |
| - Failsafe runs during integration-test/verify phases. The default config in core binds it to look for IntegrationTestsWithInternet/IntegrationTestsWithoutInternet suites. | |
| - Windows path note: Always use backslashes for file paths when referencing resources; Maven parameters still use Java FQNs for -Dtest, not file paths. | |
| - Run integration tests (if any configured in the module): | |
| - Preferred (ITs only): mvn -q -pl core -am -DskipITs=false failsafe:integration-test failsafe:verify | |
| - Run unit + IT together: mvn -q -pl core -am verify | |
| - Note: -DskipTests is honored by Failsafe as well and skips ITs; do not combine it with -DskipITs=false. | |
| - Failsafe runs during integration-test/verify phases. The default config in core binds it to look for IntegrationTestsWithInternet/IntegrationTestsWithoutInternet suites. | |
| - Windows path note: Always use backslashes for file paths when referencing resources; Maven parameters still use Java FQNs for -Dtest, not file paths. |
🤖 Prompt for AI Agents
In .junie/guidelines.md around lines 54-56, the note is misleading because
-DskipTests is honored by Failsafe too, so combining it with -DskipITs=false
won't run integration tests; update the guidance to tell readers to invoke the
Failsafe plugin goals explicitly (e.g., run failsafe:integration-test and
failsafe:verify) or to avoid using -DskipTests when you intend to run ITs and
instead rely on -DskipITs to skip only the Failsafe phase, and adjust the
example command text accordingly.
2 participants
Summary by CodeRabbit