-
Notifications
You must be signed in to change notification settings - Fork 87
Retire genUI configuration. #586
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
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.
Code Review
This pull request successfully retires the GenUiConfiguration, which simplifies the API by removing the conditional enabling of create, update, and delete actions. The changes are consistently applied throughout the codebase, including the removal of the configuration file, updates to GenUiManager and associated tools, and cleanup of test files. This is a positive simplification of the code. I have one minor formatting suggestion to ensure consistency.
| final List<AiTool<JsonMap>> availableTools = [ | ||
| if (configuration.actions.allowCreate || | ||
| configuration.actions.allowUpdate) ...[ | ||
| SurfaceUpdateTool( | ||
| handleMessage: _a2uiMessageController.add, | ||
| catalog: catalog, | ||
| configuration: configuration, | ||
| ), | ||
| BeginRenderingTool( | ||
| handleMessage: _a2uiMessageController.add, | ||
| catalogId: catalog.catalogId, | ||
| ), | ||
| ], | ||
| if (configuration.actions.allowDelete) | ||
| DeleteSurfaceTool(handleMessage: _a2uiMessageController.add), | ||
| SurfaceUpdateTool( | ||
| handleMessage: _a2uiMessageController.add, | ||
| catalog: catalog, | ||
| ), | ||
| BeginRenderingTool( | ||
| handleMessage: _a2uiMessageController.add, | ||
| catalogId: catalog.catalogId, | ||
| ), | ||
|
|
||
| DeleteSurfaceTool(handleMessage: _a2uiMessageController.add), | ||
| ...additionalTools, | ||
| ]; |
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.
There's an extra blank line in this list initialization. Running the auto-formatter will resolve this and ensure consistency with the project's coding style.
| final List<AiTool<JsonMap>> availableTools = [ | |
| if (configuration.actions.allowCreate || | |
| configuration.actions.allowUpdate) ...[ | |
| SurfaceUpdateTool( | |
| handleMessage: _a2uiMessageController.add, | |
| catalog: catalog, | |
| configuration: configuration, | |
| ), | |
| BeginRenderingTool( | |
| handleMessage: _a2uiMessageController.add, | |
| catalogId: catalog.catalogId, | |
| ), | |
| ], | |
| if (configuration.actions.allowDelete) | |
| DeleteSurfaceTool(handleMessage: _a2uiMessageController.add), | |
| SurfaceUpdateTool( | |
| handleMessage: _a2uiMessageController.add, | |
| catalog: catalog, | |
| ), | |
| BeginRenderingTool( | |
| handleMessage: _a2uiMessageController.add, | |
| catalogId: catalog.catalogId, | |
| ), | |
| DeleteSurfaceTool(handleMessage: _a2uiMessageController.add), | |
| ...additionalTools, | |
| ]; | |
| final List<AiTool<JsonMap>> availableTools = [ | |
| SurfaceUpdateTool( | |
| handleMessage: _a2uiMessageController.add, | |
| catalog: catalog, | |
| ), | |
| BeginRenderingTool( | |
| handleMessage: _a2uiMessageController.add, | |
| catalogId: catalog.catalogId, | |
| ), | |
| DeleteSurfaceTool(handleMessage: _a2uiMessageController.add), | |
| ...additionalTools, | |
| ]; |
References
- The repository style guide states that code should follow relevant style guides and use the correct auto-formatter. In this case,
dart formatwould remove the unnecessary blank line. (link)
gspencergoog
left a comment
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.

Contributes to #463