-
Notifications
You must be signed in to change notification settings - Fork 450
feat: add connections schema #1900
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes refactor the schema generation process by splitting the original functionality into two separate functions dedicated to generating settings and connections schemas. A new constant is introduced to define the filename for connection schemas. The schema generation functions now use reflection on configuration types to produce JSON schemas, with improved error handling that returns errors rather than terminating execution immediately. The main function has been updated to sequentially execute the new schema generators. In addition, the frontend has been modified to include a new endpoint mapping for the connections schema, simplifying the logic by directly assigning parsed JSON data to the schema variable. A new JSON schema file has also been added, which outlines the structure and validation rules for connection configurations across various property types. Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cmd/generateschema/main-generateschema.go (1)
37-53
: Well-structured schema generation for connections.
ThegenerateConnectionsSchema()
function mirrors the structure of the settings schema generator and maintains consistency. Consider adding unit tests that verify the generated schema aligns with expectations for various configurations.frontend/app/view/codeeditor/schemaendpoints.ts (1)
21-21
: Refine the “schema” type for better clarity.
While assigningschema = await data.json();
works, the type is declared asObject
. Static analysis warns against usingObject
as a type. Consider narrowing it to a more specific type (e.g.,Record<string, unknown>
or a custom interface) to improve type safety.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/generateschema/main-generateschema.go
(1 hunks)frontend/app/view/codeeditor/schemaendpoints.ts
(1 hunks)schema/connections.json
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/app/view/codeeditor/schemaendpoints.ts
[error] 18-18: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (8)
cmd/generateschema/main-generateschema.go (6)
18-18
: Good addition of a dedicated constant.
IntroducingWaveSchemaConnectionsFileName
as its own constant for the connections schema file improves readability and maintainability by centralizing the file path reference.
20-20
: Clear separation of responsibilities.
Extracting settings schema generation into a dedicatedgenerateSettingsSchema()
function is a clean design decision that enhances modularity and reusability.
25-26
: Returning errors instead of terminating is good.
Replacinglog.Fatalf
with an error return here is beneficial because it allows the caller to handle the error gracefully.
32-33
: Consistent error handling approach.
Continuing to return an error on file write failures, rather than terminating the process, yields better control flow.
34-35
: Proper successful completion signal.
Returningnil
when no errors occur is clear and idiomatic in Go.
55-64
: Main function orchestration is straightforward.
Invoking both schema generation functions in sequence and logging fatal errors if any occur ensures quick feedback on schema issues. This orchestrated workflow is concise and maintainable.frontend/app/view/codeeditor/schemaendpoints.ts (1)
15-15
: New endpoint mapping is consistent.
Adding the connections schema endpoint here smoothly integrates the newly generatedconnections.json
into the existing schema infrastructure.schema/connections.json (1)
1-132
: Schema definition looks thorough, but verify usage of special property names.
The newconnections.json
schema covers a wide range of properties and enforces strict validation. However, review the usage of the property"term:*"
to ensure it is intentional, given that special symbols in keys can sometimes conflict with typical JSON Schema patterns. If you plan to match multiple keys beginning withterm:
, consider usingpatternProperties
for flexibility. Also confirm that port-like fields (ssh:port
) are intentionally strings if that aligns with how they are consumed.
This adds the schema needed for the connections.json file
This adds the schema needed for the connections.json file