Skip to content

Conversation

@polina-c
Copy link
Collaborator

@polina-c polina-c commented Dec 9, 2025

Contributes to #463

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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.

Comment on lines 343 to 355
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,
];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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
  1. The repository style guide states that code should follow relevant style guides and use the correct auto-formatter. In this case, dart format would remove the unnecessary blank line. (link)

@polina-c polina-c marked this pull request as ready for review December 9, 2025 20:21
Copy link
Collaborator

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

@gspencergoog
Copy link
Collaborator

gspencergoog commented Dec 9, 2025

cc @csells - You'll need to update #583 to accomodate this.

@polina-c polina-c merged commit 061ba89 into flutter:main Dec 9, 2025
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants