Migration guide for 7#2620
Conversation
…L handling, and error tolerance
…ing YAML configuration, removed interceptors, and updated APIs
📝 WalkthroughWalkthroughAdds a new, extensive Migration Guide (Membrane 6.X → 7) introducing YAML as a first‑class configuration format while retaining XML support, enumerating removed/deprecated interceptors/plugins and updated concepts (routing, access control, logging, scripting, JMX, Java interfaces), and updates ROADMAP entries including a 7.0.4 block. Changes
Sequence Diagram(s)(omitted—documentation-only changes, no runtime control-flow modifications) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/MIGRATION-GUIDE.md`:
- Line 50: Replace the misspelled XML attribute "headeOnly" with the correct
"headerOnly" in the <log .../> example in MIGRATION-GUIDE.md so the example
reads consistently (and ensure the example aligns with the suggested alternative
that uses <log body='false'/>); search for "headeOnly" and update it to
"headerOnly" within the <log> tag.
- Line 99: The sentence is incomplete:
"HttpClientInterceptor.setAdjustHeader(boolean) has been removed. Header
adjustment is configured via HttpClientConfiguration." Update it to be a
complete, explicit sentence and point readers to where to configure it—e.g.,
replace with something like: "HttpClientInterceptor.setAdjustHeader(boolean) has
been removed; header adjustment is now configured via HttpClientConfiguration
(see the HttpClientConfiguration section below for details on the new
property/method)." Ensure the updated line references the same symbols
(HttpClientInterceptor.setAdjustHeader(boolean), HttpClientConfiguration) and
either links to or names the new configuration entry.
In `@docs/ROADMAP.md`:
- Line 37: Fix the typo in ROADMAP.md by changing "sensitve" to "sensitive" in
the sentence "Language is case sensitve: e.g. language: SPEL is not valid
according to the Schema and produces:" so the wording reads "Language is case
sensitive: e.g. language: SPEL is not valid according to the Schema and
produces:".
🧹 Nitpick comments (3)
docs/MIGRATION-GUIDE.md (3)
54-70: Add language specifiers to code blocks.The code blocks on lines 54, 62, and 68 should specify a language for proper syntax highlighting and better readability.
📝 Proposed fix
-``` +```groovy headers['X-My-Header']Returned an object of the
Headerclass. Now it returnsMap<String,Object>.Instead of:
-
+groovy
message.headers.getFirst("X-My-Header")use: -``` +```groovy headers['X-My-Header']</details> --- `74-74`: **Improve wording for clarity.** The word "delivered" is unusual in this context. Consider using "returned" for consistency with standard programming terminology. <details> <summary>📝 Proposed fix</summary> ```diff -- `headers.foo` delivered only the first value of the header `foo`. Now it returns a comma separated list of values. +- `headers.foo` returned only the first value of the header `foo`. Now it returns a comma separated list of values.
78-81: Complete the empty Groovy Expressions section.This section is currently empty. If there are no specific Groovy-related changes to document, consider removing this section entirely or add a note indicating no changes. Otherwise, please add the relevant migration information for Groovy expressions.
Do you want me to help draft content for this section based on the context from other sections, or should this section be removed?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/MIGRATION-GUIDE.mddocs/ROADMAP.md
🧰 Additional context used
🪛 LanguageTool
docs/ROADMAP.md
[grammar] ~37-~37: Ensure spelling is correct
Context: ...erantly ignore it. - Language is case sensitve: e.g. language: SPEL is not valid accord...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
docs/MIGRATION-GUIDE.md
[grammar] ~4-~4: Use a hyphen to join words.
Context: ...on 7. Membrane 7 introduces a new YAML based configuration model and a signific...
(QB_NEW_EN_HYPHEN)
[grammar] ~5-~5: Use a hyphen to join words.
Context: ... modernized internal architecture. XML based configurations remain supported, b...
(QB_NEW_EN_HYPHEN)
[grammar] ~17-~17: Use a hyphen to join words.
Context: ...ormat - Membrane 7 introduces a new YAML based configuration format that replaces...
(QB_NEW_EN_HYPHEN)
[grammar] ~30-~30: Use a hyphen to join words.
Context: ...| xmlSessionIdExtractor | Use language based session handling | Remove these e...
(QB_NEW_EN_HYPHEN)
[grammar] ~74-~74: Use a hyphen to join words.
Context: ...the header foo. Now it returns a comma separated list of values. - A nonexistin...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
docs/MIGRATION-GUIDE.md
54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
62-62: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
68-68: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (2)
- GitHub Check: Automated tests
- GitHub Check: Analyze (java)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…st for improved test performance
…ng and API changes sections
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/MIGRATION-GUIDE.md`:
- Around line 82-85: The JMX ObjectName example is truncated; update the
MIGRATION-GUIDE to show a complete ObjectName like
io.membrane-api:00=routers,name=<routerName> (or use a concrete example such as
name=MyRouter) so readers know the full key-value syntax and the placeholder to
replace when updating monitoring/CI configurations.
- Line 99: Fix the typo in the migration note: change "new configured via
`HttpClientConfiguration`" to "now configured via `HttpClientConfiguration`" in
the sentence mentioning `HttpClientInterceptor.setAdjustHeader(boolean)` so it
reads that header adjustment is now configured via `HttpClientConfiguration`.
🧹 Nitpick comments (2)
docs/MIGRATION-GUIDE.md (2)
54-70: Consider adding language identifiers to code blocks.The code blocks for scripting examples lack language specification, which affects syntax highlighting. Based on the context (SpEL/Groovy expressions), consider specifying the language:
-``` +```groovy headers['X-My-Header']--- `72-80`: **Fix section numbering inconsistency.** Sections jump from "6. Scripting" directly to "7.1. SpEL Expressions" and "7.2. Groovy Expressions" without a parent "7" section header. Consider restructuring: ```diff -## 7.1. SpEL Expressions +## 7. Expression Language Changes + +### 7.1. SpEL Expressions - `headers.foo` delivered only the first value of the header `foo`. Now it returns a comma separated list of values. - A nonexisting header like `header['x-unknown']` returned an empty string. Now it returns `null`. -## 7.2. Groovy Expressions +### 7.2. Groovy Expressions
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/src/test/java/com/predic8/membrane/core/exchangestore/AbortExchangeTest.javadocs/MIGRATION-GUIDE.md
🧰 Additional context used
🪛 LanguageTool
docs/MIGRATION-GUIDE.md
[grammar] ~4-~4: Use a hyphen to join words.
Context: ...on 7. Membrane 7 introduces a new YAML based configuration model and a signific...
(QB_NEW_EN_HYPHEN)
[grammar] ~5-~5: Use a hyphen to join words.
Context: ... modernized internal architecture. XML based configurations remain supported, b...
(QB_NEW_EN_HYPHEN)
[grammar] ~17-~17: Use a hyphen to join words.
Context: ...ormat - Membrane 7 introduces a new YAML based configuration format that replaces...
(QB_NEW_EN_HYPHEN)
[grammar] ~30-~30: Use a hyphen to join words.
Context: ...| xmlSessionIdExtractor | Use language based session handling | Remove these e...
(QB_NEW_EN_HYPHEN)
[grammar] ~74-~74: Use a hyphen to join words.
Context: ...the header foo. Now it returns a comma separated list of values. - A nonexistin...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
docs/MIGRATION-GUIDE.md
54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
62-62: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
68-68: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (2)
- GitHub Check: Automated tests
- GitHub Check: Analyze (java)
🔇 Additional comments (2)
core/src/test/java/com/predic8/membrane/core/exchangestore/AbortExchangeTest.java (1)
82-84: Verify that removing the sleep doesn't cause test flakiness.The
sleep(100)was likely allowing async exchange store updates to complete before the assertion. WhileIOUtils.copy()is synchronous for the I/O operation, theassertExchangeStoreHascheck at line 84 may depend on background processing.Note that the similar
abort()test at line 96 still retains itssleep(100)afterBodyUtil.closeConnection().If this test becomes flaky in CI, consider restoring the sleep or using a polling assertion (e.g., Awaitility) instead.
docs/MIGRATION-GUIDE.md (1)
1-13: Good migration guide structure.The migration strategy section provides a clear, actionable path for users upgrading from 6.X to 7. The recommendation to start with existing XML configuration and gradually migrate is practical.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
This pull request needs "/ok-to-test" from an authorized committer. |
…ation-guide-for-7
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/MIGRATION-GUIDE.md`:
- Around line 54-70: Add language specifiers to the three fenced code blocks
showing headers['X-My-Header'] and message.headers.getFirst("X-My-Header") so
they get proper syntax highlighting; update each triple-backtick fence that
surrounds the snippets containing headers['X-My-Header'] and
message.headers.getFirst("X-My-Header") to include the language (e.g.,
```groovy) while keeping the code content unchanged.
♻️ Duplicate comments (1)
docs/MIGRATION-GUIDE.md (1)
82-85: Complete the JMX ObjectName format example.The JMX ObjectName format ends with
name=without showing what follows. Users migrating their monitoring or CI pipelines need the complete format to update their configurations correctly.📝 Proposed fix
-The JMX ObjectName format has changed to: `io.membrane-api:00=routers, name=` +The JMX ObjectName format has changed to: `io.membrane-api:00=routers,name=<router-name>`Or provide a concrete example:
-The JMX ObjectName format has changed to: `io.membrane-api:00=routers, name=` +The JMX ObjectName format has changed to: `io.membrane-api:00=routers,name=MyRouter`
🧹 Nitpick comments (1)
docs/MIGRATION-GUIDE.md (1)
4-5: Consider hyphenating compound modifiers for improved readability (optional).Style guides recommend hyphenating compound adjectives that precede nouns. Consider:
- "YAML-based configuration" (lines 4, 17)
- "XML-based configurations" (line 5)
- "language-based session handling" (line 30)
- "comma-separated list" (line 74)
📝 Proposed style improvements
-Membrane 7 introduces a new YAML based configuration model and a significantly modernized internal architecture. -XML based configurations remain supported, but some legacy features and APIs have been removed. +Membrane 7 introduces a new YAML-based configuration model and a significantly modernized internal architecture. +XML-based configurations remain supported, but some legacy features and APIs have been removed.-- Membrane 7 introduces a new YAML based configuration format that replaces most use cases of `proxies.xml`. +- Membrane 7 introduces a new YAML-based configuration format that replaces most use cases of `proxies.xml`.-| `xmlSessionIdExtractor` | Use language based session handling | +| `xmlSessionIdExtractor` | Use language-based session handling |-- `headers.foo` delivered only the first value of the header `foo`. Now it returns a comma separated list of values. +- `headers.foo` delivered only the first value of the header `foo`. Now it returns a comma-separated list of values.Also applies to: 17-17, 30-30, 74-74
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/MIGRATION-GUIDE.md
🧰 Additional context used
🪛 LanguageTool
docs/MIGRATION-GUIDE.md
[grammar] ~4-~4: Use a hyphen to join words.
Context: ...on 7. Membrane 7 introduces a new YAML based configuration model and a signific...
(QB_NEW_EN_HYPHEN)
[grammar] ~5-~5: Use a hyphen to join words.
Context: ... modernized internal architecture. XML based configurations remain supported, b...
(QB_NEW_EN_HYPHEN)
[grammar] ~17-~17: Use a hyphen to join words.
Context: ...ormat - Membrane 7 introduces a new YAML based configuration format that replaces...
(QB_NEW_EN_HYPHEN)
[grammar] ~30-~30: Use a hyphen to join words.
Context: ...| xmlSessionIdExtractor | Use language based session handling | Remove these e...
(QB_NEW_EN_HYPHEN)
[grammar] ~74-~74: Use a hyphen to join words.
Context: ...the header foo. Now it returns a comma separated list of values. - A nonexistin...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
docs/MIGRATION-GUIDE.md
54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
62-62: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
68-68: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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)
docs/MIGRATION-GUIDE.md (4)
7-13: Clear and actionable migration strategy.The step-by-step migration approach is well-structured and practical. It appropriately recommends incremental migration and testing, which reduces risk for users upgrading production systems.
22-32: Helpful table format for deprecated features.The table clearly maps removed interceptors to their modern replacements, making it easy for users to understand what changes are needed in their configurations.
34-50: Well-structured before/after examples.The XML examples for internal routing and logging changes provide clear before/after comparisons, making it straightforward for users to update their configurations.
87-100: Comprehensive Java API changes documentation.The Java interface changes are well-documented with clear explanations and a helpful code example showing the before/after comparison for the
HttpClient#callmethod signature change.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…uotation style test(exchangestore): uncomment sleep in AbortExchangeTest to restore intended behavior
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/MIGRATION-GUIDE.md`:
- Around line 60-62: The Groovy code blocks in the Groovy Expressions section
lack language specifiers; update each code fence that shows expressions like
headers.getFirst("X-My-Header") and headers['X-My-Header'] to include the
"groovy" specifier (i.e., change ``` to ```groovy) so documentation viewers
render proper syntax highlighting for those expressions.
♻️ Duplicate comments (2)
docs/MIGRATION-GUIDE.md (2)
50-50: Use double quotes for XML attributes per XML conventions.XML style guidelines recommend double quotes for attribute values. This aligns with standard XML formatting and improves consistency throughout the documentation.
Based on past review comments.
📝 Proposed fix
-Instead of `<log headerOnly='true'/>` use `<log body='false'/>`. +Instead of `<log headerOnly="true"/>` use `<log body="false"/>`.
77-77: Complete the JMX ObjectName format example.The ObjectName format is truncated and ends with
name=without showing what follows. Users migrating their monitoring and CI pipelines need the complete format to properly update their configurations.Based on past review comments.
📝 Proposed fix
-The JMX ObjectName format has changed to: `io.membrane-api:00=routers, name=` +The JMX ObjectName format has changed to: `io.membrane-api:00=routers, name=<router-name>`Or provide a concrete example:
-The JMX ObjectName format has changed to: `io.membrane-api:00=routers, name=` +The JMX ObjectName format has changed to: `io.membrane-api:00=routers, name=MyRouter`
🧹 Nitpick comments (4)
docs/MIGRATION-GUIDE.md (4)
4-5: Consider hyphenating compound adjectives for consistency.The phrases "YAML based" and "XML based" function as compound adjectives modifying nouns. Standard English style guides recommend hyphenating them ("YAML-based", "XML-based") for clarity and professionalism.
📝 Proposed fix
-Membrane 7 introduces a new YAML based configuration model and a significantly modernized internal architecture. -XML based configurations remain supported, but some legacy features and APIs have been removed. +Membrane 7 introduces a new YAML-based configuration model and a significantly modernized internal architecture. +XML-based configurations remain supported, but some legacy features and APIs have been removed.
17-17: Apply consistent hyphenation.Similar to lines 4-5, "YAML based" should be hyphenated.
📝 Proposed fix
-- Membrane 7 introduces a new YAML based configuration format that replaces most use cases of `proxies.xml`. +- Membrane 7 introduces a new YAML-based configuration format that replaces most use cases of `proxies.xml`.
30-30: Apply consistent hyphenation in table."language based" should be hyphenated as "language-based" when used as a compound adjective.
📝 Proposed fix
-| `xmlSessionIdExtractor` | Use language based session handling. See examples/loadbalancing/4-session | +| `xmlSessionIdExtractor` | Use language-based session handling. See examples/loadbalancing/4-session |
72-72: Apply consistent hyphenation."comma separated" should be hyphenated as "comma-separated" when used as a compound adjective.
📝 Proposed fix
-- `headers.foo` delivered only the first value of the header `foo`. Now it returns a comma separated list of values. +- `headers.foo` delivered only the first value of the header `foo`. Now it returns a comma-separated list of values.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/MIGRATION-GUIDE.md
🧰 Additional context used
🪛 LanguageTool
docs/MIGRATION-GUIDE.md
[grammar] ~4-~4: Use a hyphen to join words.
Context: ...on 7. Membrane 7 introduces a new YAML based configuration model and a signific...
(QB_NEW_EN_HYPHEN)
[grammar] ~5-~5: Use a hyphen to join words.
Context: ... modernized internal architecture. XML based configurations remain supported, b...
(QB_NEW_EN_HYPHEN)
[grammar] ~17-~17: Use a hyphen to join words.
Context: ...ormat - Membrane 7 introduces a new YAML based configuration format that replaces...
(QB_NEW_EN_HYPHEN)
[grammar] ~30-~30: Use a hyphen to join words.
Context: ...| xmlSessionIdExtractor | Use language based session handling. See examples/loa...
(QB_NEW_EN_HYPHEN)
[grammar] ~72-~72: Use a hyphen to join words.
Context: ...the header foo. Now it returns a comma separated list of values. - A nonexistin...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
docs/MIGRATION-GUIDE.md
60-60: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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 (1)
docs/MIGRATION-GUIDE.md (1)
80-92: LGTM! Java interface changes are clearly documented.The breaking changes to Java interfaces are well-documented with clear before/after examples. The code snippet showing the HttpClient#call signature change is particularly helpful for developers updating their code.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/MIGRATION-GUIDE.md (1)
156-156: Remove extraneous quotation mark.The line ends with a stray quotation mark (
")) that appears to be a typo.📝 Proposed fix
-Default naming scheme for `<serviceProxys>` has changed. This might affect existing filters in log aggregation systems and/or monitoring dashboards, e.g. in Prometheus/Grafana.") +Default naming scheme for `<serviceProxys>` has changed. This might affect existing filters in log aggregation systems and/or monitoring dashboards, e.g. in Prometheus/Grafana.
♻️ Duplicate comments (3)
docs/MIGRATION-GUIDE.md (3)
60-68: Add language specifiers to Groovy code blocks.The code blocks are missing language specifiers, preventing proper syntax highlighting. Based on past review comments and the section title "Groovy Expressions," these should be marked as
groovy.📝 Proposed fix
-``` +```groovy headers.getFirst("X-My-Header")```diff -``` +```groovy headers['X-My-Header']</details> --- `76-78`: **Complete the JMX ObjectName format example.** The JMX ObjectName format appears truncated - it ends with `name=` without showing the value placeholder or an example. Users migrating monitoring and CI pipelines need the complete format. <details> <summary>📝 Proposed fix</summary> ```diff -The JMX ObjectName format has changed to: `io.membrane-api:00=routers, name=` +The JMX ObjectName format has changed to: `io.membrane-api:00=routers,name=<router-name>`Or provide a concrete example:
-The JMX ObjectName format has changed to: `io.membrane-api:00=routers, name=` +The JMX ObjectName format has changed to: `io.membrane-api:00=routers,name=default`
92-92: Provide specific guidance for HttpClientConfiguration usage.The sentence mentions that header adjustment is now configured via
HttpClientConfigurationbut doesn't specify which method, property, or approach to use. Users migrating their code need concrete guidance on the replacement API.📝 Suggested improvement
Consider adding specific details:
-- `HttpClientInterceptor.setAdjustHeader(boolean)` has been removed. Header adjustment is now configured via `HttpClientConfiguration`. +- `HttpClientInterceptor.setAdjustHeader(boolean)` has been removed. Header adjustment is now configured via `HttpClientConfiguration` using the `setAdjustHostHeader(boolean)` method.Or reference documentation:
-- `HttpClientInterceptor.setAdjustHeader(boolean)` has been removed. Header adjustment is now configured via `HttpClientConfiguration`. +- `HttpClientInterceptor.setAdjustHeader(boolean)` has been removed. Header adjustment is now configured via `HttpClientConfiguration` (see the `HttpClientConfiguration` JavaDoc for available configuration options).#!/bin/bash # Description: Find the actual method name in HttpClientConfiguration for header adjustment # Search for HttpClientConfiguration class definition and methods related to header adjustment echo "=== Searching for HttpClientConfiguration methods ===" ast-grep --pattern 'class HttpClientConfiguration { $$$ }' echo -e "\n=== Searching for adjustHeader related methods ===" rg -nP --type=java -C5 'class HttpClientConfiguration' | head -50 echo -e "\n=== Searching for header adjustment setters ===" rg -nP --type=java 'setAdjust.*[Hh]eader|adjustHeader.*\(' -C3
🧹 Nitpick comments (5)
docs/MIGRATION-GUIDE.md (5)
4-5: Consider hyphenating compound adjectives for consistency.Static analysis suggests using hyphens in compound adjectives like "YAML-based" and "XML-based" for better readability and grammatical consistency.
📝 Proposed refinement
-Membrane 7 introduces a new YAML based configuration model and a significantly modernized internal architecture. -XML based configurations remain supported, but some legacy features and APIs have been removed. +Membrane 7 introduces a new YAML-based configuration model and a significantly modernized internal architecture. +XML-based configurations remain supported, but some legacy features and APIs have been removed.
38-45: Use double quotes for XML attributes.Based on past review feedback, XML attributes should use double quotes (
") instead of single quotes (') for consistency with XML conventions.📝 Proposed fix
-<target url="service:a"/> +<target url="service:a"/>-<target url="internal://a"/> +<target url="internal://a"/>
50-50: Use double quotes for XML attributes.Based on past review feedback, XML attributes should use double quotes (
") instead of single quotes (') for consistency with XML conventions.📝 Proposed fix
-Instead of `<log headerOnly="true"/>` use `<log body="false"/>`. +Instead of `<log headerOnly="true"/>` use `<log body="false"/>`.
70-70: Remove period after section number for consistency.Section numbering format is inconsistent - this section uses "6.2." with a period, while other sections use numbers without periods (e.g., "6.1 Groovy Expressions").
📝 Proposed fix
-## 6.2. SpEL Expressions +## 6.2 SpEL Expressions
138-138: Use double quotes for XML attributes.Consistent with earlier feedback, XML attributes should use double quotes (
") instead of single quotes (').📝 Proposed fix
-Instead of `headerOnly="true"` use `body="false"` in the `<log>` plugin. +Instead of `headerOnly="true"` use `body="false"` in the `<log>` plugin.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/MIGRATION-GUIDE.md
🧰 Additional context used
🪛 LanguageTool
docs/MIGRATION-GUIDE.md
[grammar] ~4-~4: Use a hyphen to join words.
Context: ...on 7. Membrane 7 introduces a new YAML based configuration model and a signific...
(QB_NEW_EN_HYPHEN)
[grammar] ~5-~5: Use a hyphen to join words.
Context: ... modernized internal architecture. XML based configurations remain supported, b...
(QB_NEW_EN_HYPHEN)
[grammar] ~17-~17: Use a hyphen to join words.
Context: ...ormat - Membrane 7 introduces a new YAML based configuration format that replaces...
(QB_NEW_EN_HYPHEN)
[grammar] ~30-~30: Use a hyphen to join words.
Context: ...| xmlSessionIdExtractor | Use language based session handling. See examples/loa...
(QB_NEW_EN_HYPHEN)
[grammar] ~72-~72: Use a hyphen to join words.
Context: ...the header foo. Now it returns a comma separated list of values. - A nonexistin...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
docs/MIGRATION-GUIDE.md
60-60: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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 (1)
docs/MIGRATION-GUIDE.md (1)
22-32: No changes needed to the migration guide at these lines. The ACL interceptor remains actively supported as AccessControlInterceptor V2 (refactored from V1, not removed), API key scopes are actively used in multiple store implementations, and telekomSMSTokenProvider does not have an actual implementation in the codebase (only mentioned in documentation). The table's current list of removed interceptors is accurate.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…ceptors and replacements
…onfiguration updates
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/MIGRATION-GUIDE.md`:
- Around line 110-112: The fenced code block showing headers['X-My-Header']
lacks a language specifier; update the triple-backtick fence around the Groovy
expression (the code snippet containing headers['X-My-Header']) to include
"groovy" (i.e., change ``` to ```groovy) so the Groovy Expressions section is
properly syntax-highlighted.
- Around line 96-118: Update the subsection headings under "## 7. Scripting":
rename "### 6.1 Groovy Expressions" to "### 7.1 Groovy Expressions" and change
"## 6.2. SpEL Expressions" to a third-level heading "### 7.2. SpEL Expressions"
so both subsections use the correct 7.x numbering and match the same heading
level; ensure the text that references these headings uses the new names.
♻️ Duplicate comments (1)
docs/MIGRATION-GUIDE.md (1)
119-122: Complete the JMX ObjectName format example.The JMX ObjectName format example ends with
name=without showing what follows. Users migrating their monitoring/CI pipelines need the complete format to update their configurations correctly.📝 Proposed fix
-The JMX ObjectName format has changed to: `io.membrane-api:00=routers, name=` +The JMX ObjectName format has changed to: `io.membrane-api:00=routers,name=<router-name>`Or provide a concrete example:
-The JMX ObjectName format has changed to: `io.membrane-api:00=routers, name=` +The JMX ObjectName format has changed to: `io.membrane-api:00=routers,name=MyRouter`
🧹 Nitpick comments (1)
docs/MIGRATION-GUIDE.md (1)
52-54: Consider adding backticks for technical terms for consistency.Based on past review feedback patterns, technical terms like
acl.xmlandaccessControltypically use backticks throughout the document. Consider updating:
- "external
acl.xml" (already correct)- "
accessControl" when referring to the configuration elementBased on learnings from past review comments.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/MIGRATION-GUIDE.md
🧰 Additional context used
🪛 LanguageTool
docs/MIGRATION-GUIDE.md
[grammar] ~4-~4: Use a hyphen to join words.
Context: ...on 7. Membrane 7 introduces a new YAML based configuration model and a signific...
(QB_NEW_EN_HYPHEN)
[grammar] ~5-~5: Use a hyphen to join words.
Context: ... modernized internal architecture. XML based configurations remain supported, b...
(QB_NEW_EN_HYPHEN)
[grammar] ~17-~17: Use a hyphen to join words.
Context: ...ormat - Membrane 7 introduces a new YAML based configuration format that replaces...
(QB_NEW_EN_HYPHEN)
[grammar] ~44-~44: Use a hyphen to join words.
Context: ...| xmlSessionIdExtractor | Use language based session handling. See examples/loa...
(QB_NEW_EN_HYPHEN)
[grammar] ~116-~116: Use a hyphen to join words.
Context: ...the header foo. Now it returns a comma separated list of values. - A nonexistin...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
docs/MIGRATION-GUIDE.md
110-110: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/ROADMAP.md`:
- Around line 58-68: Fix typos and markdown list structure in the ROADMAP
entries: correct misspellings like "ressources" -> "resources" and "Old an
unused" -> "Old and unused", normalize verbs (e.g., "Register JSON Schema" ->
"Register JSON Schema for YAML at <https://www.schemastore.org>" or convert the
URL into a proper markdown link), ensure nested bullets under "accessControl:"
are indented consistently as sub-items, treat items that start with keywords
like "if:" or "Remove GroovyTemplateInterceptor (Not Template Interceptor)" as
plain list entries or wrap keywords in backticks for clarity, and run
markdownlint rules to confirm list formatting and URL handling (refer to the
exact strings "Migrate deprecated finally to try with ressources",
"accessControl:", "Register JSON Schema for YAML at:
https://www.schemastore.org", and "Remove GroovyTemplateInterceptor (Not
Template Interceptor)").
🧹 Nitpick comments (1)
docs/MIGRATION-GUIDE.md (1)
4-5: Hyphenate compound adjectives for readability.Consider “YAML-based”, “XML-based”, “language-based”, and “comma-separated”.
✍️ Suggested wording
-Membrane 7 introduces a new YAML based configuration model and a significantly modernized internal architecture. -XML based configurations remain supported, but some legacy features and APIs have been removed. +Membrane 7 introduces a new YAML-based configuration model and a significantly modernized internal architecture. +XML-based configurations remain supported, but some legacy features and APIs have been removed.-- Membrane 7 introduces a new YAML based configuration format that replaces most use cases of `proxies.xml`. +- Membrane 7 introduces a new YAML-based configuration format that replaces most use cases of `proxies.xml`.-| `xmlSessionIdExtractor` | Use language based session handling. See examples/loadbalancing/4-session | +| `xmlSessionIdExtractor` | Use language-based session handling. See examples/loadbalancing/4-session |-- `headers.foo` delivered only the first value of the header `foo`. Now it returns a comma separated list of values. +- `headers.foo` delivered only the first value of the header `foo`. Now it returns a comma-separated list of values.Also applies to: 17-18, 44-44, 117-117
3 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.