Check Dart formatting in all packages and in tool/#9873
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds formatting checks using dart format to several CI test scripts (package_tests.sh and tool_tests.sh) before running tests. The reviewer recommends invoking dart format directly instead of using the Flutter wrapper script $(dirname $(which flutter))/dart to avoid potential wrapper script issues and simplify the code, since the direct Dart SDK binary is already prioritized in the PATH.
| # Here, we use the dart instance from the flutter SDK. | ||
| $(dirname $(which flutter))/dart format --output=none --set-exit-if-changed . |
There was a problem hiding this comment.
[CONCERN] Running $(dirname $(which flutter))/dart invokes the Dart wrapper script from flutter/bin, which can print "Waiting for another flutter command..." at inopportune times. As documented in setup.sh, the PATH is already configured to prioritize the direct Dart SDK binary. We should run dart format directly instead, which also makes the explanatory comment unnecessary.
| # Here, we use the dart instance from the flutter SDK. | |
| $(dirname $(which flutter))/dart format --output=none --set-exit-if-changed . | |
| dart format --output=none --set-exit-if-changed . |
| # Note that this will _not_ test any tests in nested directories, if we add | ||
| # any. |
There was a problem hiding this comment.
hmm this seems like a bug? why aren't we running flutter test test/
There was a problem hiding this comment.
I'm not sure. I can fix it in a separate PR.
kenzieschmoll
left a comment
kenzieschmoll
left a comment
There was a problem hiding this comment.
a couple comments but generally lgtm
2 participants
No description provided.