Skills/add dubbo overview skill - #16388
Conversation
Updated the token limit for SKILL.md files from 2,000 to 3,000 tokens.
Updated the SKILL.md file to include a detailed description of the dubbo-overview skill, modified license information, and adjusted various configuration settings for Dubbo 3.
Updated the SKILL.md file to clarify RPC protocols, registry support, and QoS port conflict resolution.
Updated README.md to clarify registration modes and added details for new modules.
Added new skills for metrics, REST, MCP server, and native image support.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 3.3 #16388 +/- ##
============================================
Coverage 60.87% 60.88%
+ Complexity 11766 15 -11751
============================================
Files 1953 1953
Lines 89273 89273
Branches 13473 13473
============================================
+ Hits 54346 54350 +4
+ Misses 29333 29332 -1
+ Partials 5594 5591 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| gRPC-compatible, and works through standard API gateways. New services | ||
| should use `tri` unless they need backward TCP compatibility. | ||
|
|
||
| **5. Omitting MetadataCenter during Dubbo 2→3 migration** |
There was a problem hiding this comment.
During mixed Dubbo 2/3 deployments, providers must retain interface-level registrations using register-mode=all; Dubbo 2 consumers cannot interpret application-level registrations or service-name mappings. Following the current guidance could cause No provider available. The same misconception appears at L90-L92, L160, and README L33. See the official migration guide.
| mode when you need compatibility with existing Dubbo 2 consumers that | ||
| have not been upgraded. | ||
|
|
||
| **8. QoS port conflict when running multiple Dubbo apps locally** |
There was a problem hiding this comment.
QosProtocolWrapper defaults qos.check to false, catches the bind exception, and only throws when QoS checking is enabled on the provider. The text should state that the QoS server fails to start and logs a warning, while the application normally continues.
|
|
||
| **6. Confusing Registry Center with Config Center** | ||
| Registry = where services are. Config = how services behave. | ||
| A service can use Nacos as both, but they are configured separately under |
There was a problem hiding this comment.
The statement that registry-only configuration does not enable dynamic configuration is the opposite of the current 3.3 behavior. When no Config Center is explicit, DefaultApplicationDeployer.useRegistryAsConfigCenterIfNecessary() synthesizes one from a supported registry whenever use-as-config-center is unset or true; Nacos and Zookeeper therefore enable this reuse for the sample above. Please document the default reuse and the use-as-config-center: false opt-out instead, otherwise the skill will teach assistants to misdiagnose working governance configuration.
RainYuY
left a comment
RainYuY
left a comment
There was a problem hiding this comment.
Re-reviewed at f7e49cb. The latest update only merges 3.3; git diff 0ce4bb6..f7e49cb -- skills is empty, so both existing blocking comments remain unaddressed: (1) mixed Dubbo 2/3 migration must retain interface-level registration, because a Metadata Center does not make Dubbo 2 consumers understand application-level registrations; and (2) a QoS bind conflict only logs a warning and lets the application continue by default because qos.check defaults to false. I found one additional configuration-center correctness issue inline. Requesting changes until all three are corrected.
Clarified migration requirements from Dubbo 2 to Dubbo 3, emphasizing the need for a Metadata Center and the importance of maintaining the default register-mode. Updated sections on service discovery and QoS port conflicts.
Updated README.md to clarify migration steps and project details.
3 participants
What is the purpose of the change?
Checklist