Skip to content

update and (re)enable assisted code reviews#8816

Merged
pq merged 4 commits into
mainfrom
pq-enable-config
Feb 20, 2026
Merged

update and (re)enable assisted code reviews#8816
pq merged 4 commits into
mainfrom
pq-enable-config

Conversation

@pq
Copy link
Copy Markdown
Collaborator

@pq pq commented Feb 19, 2026

Updates config to be inline with flutter's (except where noted) and enables reviews on pull requests.


A sample review provoked by a bogus TODO and system out.

image

Note how it's reading from our style guide and recommending our bespoke logger.

Closes: #8815.


  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

@pq
Copy link
Copy Markdown
Collaborator Author

pq commented Feb 19, 2026

/gemini review

Copy link
Copy Markdown
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

The pull request updates and re-enables assisted code reviews. The primary change involves adding a System.out.println statement and a TODO comment in FlutterUtils.java. My review focuses on addressing these additions in accordance with the provided style guide.

Comment thread src/io/flutter/FlutterUtils.java Outdated
Comment on lines +97 to +98
// TODO: remove this later
System.out.println("Testing the bot");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

[CONCERN] The use of System.out.println is discouraged for logging in IntelliJ Platform plugins. Please use the IntelliJ SDK's built-in logger (com.intellij.openapi.diagnostic.Logger) or io.flutter.logging.PluginLogger instead. This ensures consistent logging practices and better integration with the IDE's logging system.

Additionally, the TODO comment should be justified or removed if it's not a temporary debugging statement. If it's a temporary debugging statement, it should be removed before merging.

Suggested change
// TODO: remove this later
System.out.println("Testing the bot");
// TODO: remove this later
// LOG.info("Testing the bot");
References
  1. Reject any use of System.out.println or System.err.println for logging. Enforce the use of the IntelliJ SDK's built-in logger: com.intellij.openapi.diagnostic.Logger or our own: io.flutter.logging.PluginLogger. (link)

@pq pq requested review from cj-radcliff and helin24 February 19, 2026 23:45
@cj-radcliff
Copy link
Copy Markdown
Collaborator

This looks pretty good to me. I think we should keep it enabled a bit and watch them regularly.

Copy link
Copy Markdown
Collaborator

@cj-radcliff cj-radcliff left a comment

Choose a reason for hiding this comment

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

LGTM

@pq
Copy link
Copy Markdown
Collaborator Author

pq commented Feb 20, 2026

This looks pretty good to me. I think we should keep it enabled a bit and watch them regularly.

Thanks and agreed 100%!

@pq pq merged commit 9fef30b into main Feb 20, 2026
8 checks passed
@pq pq deleted the pq-enable-config branch February 20, 2026 16:42
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.

[infra] Update Gemini Code Assist config (and re-enable)

2 participants