Repository navigation
JSON schema: move the element id mapping to docs.yaml and drop the legacy "id" keyword - #3603
Conversation
…egacy "id" keyword
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Remove legacy schema IDs annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java, annot/src/main/java/com/predic8/membrane/annot/generator/kubernetes/model/AbstractSchema.java, distribution/src/test/java/com/predic8/membrane/examples/ApisYamlSchemaValidationTest.java |
Parser schemas are no longer tagged with element IDs, and AbstractSchema no longer serializes an id. The validation test no longer strips id keywords before creating the schema registry. |
Record schema keys in YAML documentation annot/src/main/java/com/predic8/membrane/annot/generator/YamlDocsGenerator.java |
The documentation format version changes to 2. Collection now includes elements without documentation and records each element’s JSON Schema definition key. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Feature
Possibly related issues
membrane/api-gateway#3602— The issue describes removing legacyidfields from generated JSON Schema definitions and adding the definition-to-element mapping asjsonSchemaDefindocs.yaml.
Suggested reviewers: predic8
Merge Risk: 
Minimal · up to 41cc5
-
No actionable merge-blocking risk was established by the reviewed changes.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Title check
-
Explanation
The title names two changes: moving the element id mapping to docs.yaml and dropping the legacy "id" keyword. The raw summary shows the id field and id(String) setter removed from AbstractSchema, the tagElementId id assignment removed from JsonSchemaGenerator, and stripLegacyIdKeyword removed from the test. It also shows YamlDocsGenerator changed to version 2 and emitting every element with its schema key from getXSDTypeName. The summary does not clearly show a docs.yaml mapping being added, so the title is only partly supported. The title still refers to real, central parts of the change, so it passes.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 5 functions across 2 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit checks the schema rows,
No legacy id remains to show.
Each element gets its key in view,
The YAML lists the undocumented too.
I nibble clover, then hop through.
Comment @coderabbitai help to get the list of available commands.
|
/ok-to-test |
|
This pull request needs "/ok-to-test" from an authorized committer. |
Summary by CodeRabbit