Add support for dart formatter #57075
Conversation
| } | ||
|
|
||
| Future<int> _runDartFormat({required bool fixing}) async { | ||
| final List<String> filesToCheck = await getFileList(<String>['*.dart']); |
There was a problem hiding this comment.
This is a nit:
// Todo: we probably should just do this on single list of files rather than forking X processes.
- dart formatter is pretty quick
- engineers probably aren't going to make huge changes
- most of the time is probably spent in process start
There was a problem hiding this comment.
I had this modeled after the other language formatters in this file, which all use this pattern. It has the advantage of allowing us to print a nice diff between expected formatting vs. actual formatting. It is probably faster to just pass the list of files directly to the formatter.
There was a problem hiding this comment.
Printing a nice diff seems like a good motivation to follow the same pattern.
There was a problem hiding this comment.
I don't think we need to print a nice diff. We just need to tell people the files that are failing format checking. They should be running et format before they put the PR up (which in the engine's case is true).
| }) : super( | ||
| repoDir: repoDir, | ||
| ) { | ||
| // $ENGINE/flutter/third_party/dart/tools/sdks/dart-sdk/bin/dart |
There was a problem hiding this comment.
I think this is correct, but FYI this is the prebuilt dev channel Dart SDK, and not the prebuilt Dart SDK whose version matches the hash in the DEPS file.
There was a problem hiding this comment.
@zanderso For my own understanding: What's the difference between the two and where is the version of either of these specified?
There was a problem hiding this comment.
The versions are specified in the DEPS file:
The prebuilt dev channel SDK is here:
Line 431 in 4d00124
vs.
here we download a prebuilt Dart SDK that matches the version of the Dart SDK source that is pulled into the engine:
Line 435 in 4d00124
The version of the Dart SDK that the framework will be using is the second one.
| } | ||
|
|
||
| Future<int> _runDartFormat({required bool fixing}) async { | ||
| final List<String> filesToCheck = await getFileList(<String>['*.dart']); |
There was a problem hiding this comment.
Printing a nice diff seems like a good motivation to follow the same pattern.
goderbauer
force-pushed
the
dartFormatter
branch
from
b65140a to
19f21f9
Compare
goderbauer
added
the
autosubmit
…160061) flutter/engine@3a641b6...a8f7556 2024-12-11 goderbauer@google.com Add support for dart formatter (flutter/engine#57075) 2024-12-10 skia-flutter-autoroll@skia.org Roll Skia from d541f1aa0c9b to e4d2c3dbb848 (2 revisions) (flutter/engine#57106) 2024-12-10 skia-flutter-autoroll@skia.org Roll Dart SDK from dd92932823d1 to f863f0b43625 (1 revision) (flutter/engine#57104) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC jonahwilliams@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Currently it is off by default since we haven't migrated any files over to the new format. To try it out, run `dart ci/bin/format.dart -c dart -f`. Once we turn it on by default `et` will automatically format dart files as it calls into `format.dart`.
3 participants
Currently it is off by default since we haven't migrated any files over to the new format. To try it out, run
dart ci/bin/format.dart -c dart -f.Once we turn it on by default
etwill automatically format dart files as it calls intoformat.dart.