Use 'et format' in CI. Check formatting of all files in CI #50810

zanderso · 2024-02-21T04:06:14Z

This PR changes the format check on CI to use the command added in #50747.

Additionally, while making this change, I noticed that the CI check was not checking the formatting of all files, and that as a result, files were present in the repo with incorrect formatting. I have fixed the formatting and fixed the check to always check all files.

johnmccutchan

RSLGTM

loic-sharma · 2024-02-21T17:11:00Z

impeller/renderer/backend/gles/proc_table_gles.cc

  if (!error_fn) {
-    VALIDATION_LOG << "Could not resolve "
-                   << "glGetError";
+    VALIDATION_LOG << "Could not resolve " << "glGetError";


Super nit, feel free to skip:

Suggested change

VALIDATION_LOG << "Could not resolve " << "glGetError";

VALIDATION_LOG << "Could not resolve glGetError";

loic-sharma · 2024-02-21T17:11:22Z

impeller/renderer/backend/gles/proc_table_gles.h

        FML_LOG(ERROR) << "GL Error " << GLErrorToString(error) << "(" << error
-                       << ")"
-                       << " encountered on call to " << name;
+                       << ")" << " encountered on call to " << name;


Super nit, feel free to skip:

Suggested change

<< ")" << " encountered on call to " << name;

<< ") encountered on call to " << name;

loic-sharma · 2024-02-21T17:12:22Z

lib/ui/window/viewport_metrics.cc

     << a.physical_view_inset_right << "R " << a.physical_view_inset_bottom
-     << "B " << a.physical_view_inset_left << "L] "
-     << "Gesture Insets: [" << a.physical_system_gesture_inset_top << "T "
+     << "B " << a.physical_view_inset_left << "L] " << "Gesture Insets: ["


Suggested change

<< "B " << a.physical_view_inset_left << "L] " << "Gesture Insets: ["

<< "B " << a.physical_view_inset_left << "L] Gesture Insets: ["

loic-sharma · 2024-02-21T17:12:57Z

shell/platform/darwin/ios/framework/Source/FlutterDartVMServicePublisher.mm

                   << "to the 'NSBonjourServices' key in your Info.plist for the Debug/"
-                   << "Profile configurations. "
-                   << "For more information, see "
+                   << "Profile configurations. " << "For more information, see "


Suggested change

<< "Profile configurations. " << "For more information, see "

<< "Profile configurations. For more information, see "

loic-sharma · 2024-02-21T17:14:02Z

shell/platform/fuchsia/flutter/focus_delegate_unittests.cc

  std::ostringstream message;
-  message << "{"
-          << "    \"method\":\"View.focus.request\","
+  message << "{" << "    \"method\":\"View.focus.request\","


Hm this seems worse for readability. Should we opt out of formatting for JSON strings?

loic-sharma · 2024-02-21T17:15:05Z

shell/platform/fuchsia/flutter/platform_view.cc

-  out << "{"
-      << "\"method\":\"View.viewStateChanged\","
-      << "\"args\":{"
+  out << "{" << "\"method\":\"View.viewStateChanged\"," << "\"args\":{"


Same comment for JSON string readability in this file

loic-sharma · 2024-02-21T17:15:37Z

shell/platform/fuchsia/flutter/pointer_injector_delegate_unittest.cc

    std::ostringstream message;
-    message << "{"
-            << "    \"method\":\""
+    message << "{" << "    \"method\":\""


Same comment for JSON string readability in this file

loic-sharma · 2024-02-21T17:16:03Z

shell/platform/fuchsia/flutter/tests/integration/utils/screenshot.cc

 std::ostream& operator<<(std::ostream& stream, const Pixel& pixel) {
-  return stream << "{Pixel:"
-                << " r:" << static_cast<unsigned int>(pixel.red)
+  return stream << "{Pixel:" << " r:" << static_cast<unsigned int>(pixel.red)


Suggested change

return stream << "{Pixel:" << " r:" << static_cast<unsigned int>(pixel.red)

return stream << "{Pixel: r:" << static_cast<unsigned int>(pixel.red)

loic-sharma · 2024-02-21T17:16:19Z

shell/platform/fuchsia/flutter/tests/platform_view_unittest.cc

-                      << "    \"focusable\":true"
-                      << "  }"
-                      << "}";
+  create_view_message << "{" << "  \"method\":\"View.create\","


Same comment for JSON string readability in this file

loic-sharma

Excellent catch on the formatting, thanks for doing this!

zanderso · 2024-02-21T17:38:05Z

Sorry, I'll have to skip the nits to avoid fighting with the formatter and getting this landed without merge conflicts. I suspect that folks working in these files will be able to handle cleanups that are adjacent to other changes.

…lutter/engine#50810)

…143875) flutter/engine@52ffcaa...bf5c003 2024-02-21 [email protected] Roll Skia from 9d86359b5fe8 to 8fa858855820 (15 revisions) (flutter/engine#50827) 2024-02-21 [email protected] Add the `scenario_app` `'solid_blue'` golden to the Android test suite (flutter/engine#50801) 2024-02-21 [email protected] Ignore EOF newline characters and added tests to `dir_contents_diff` tool (flutter/engine#50805) 2024-02-21 [email protected] Make the GL context current in EmbedderSurfaceGLImpeller before creating the GPU surface (flutter/engine#50807) 2024-02-21 [email protected] Fail engine post-submit on skia-gold comparions. (flutter/engine#50826) 2024-02-21 [email protected] Remove WindowManager reflection in SingleViewPresentation.java (flutter/engine#49996) 2024-02-21 [email protected] [Impeller] applied the lerp hack to blur (roughly 2x speedup?) (flutter/engine#50790) 2024-02-21 [email protected] Migrate the Fuchsia embedder to the Dart_RecordTimelineEvent API (flutter/engine#50823) 2024-02-21 [email protected] Hook ImageReaderSurfaceProducer to the onTrimMemory listener interface (flutter/engine#50792) 2024-02-21 [email protected] Fix the local-only lint errors due to an unexpected `GeneratedPluginRegistrant.java` (flutter/engine#50795) 2024-02-21 [email protected] [Impeller] cache onscreen render targets. (flutter/engine#50751) 2024-02-21 [email protected] Use 'et format' in CI. Check formatting of all files in CI (flutter/engine#50810) 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 [email protected],[email protected],[email protected] 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

github-actions bot added platform-ios affects: desktop platform-linux platform-windows platform-fuchsia e: impeller labels Feb 21, 2024

zanderso requested review from johnmccutchan, loic-sharma and matanlurey February 21, 2024 15:55

johnmccutchan approved these changes Feb 21, 2024

View reviewed changes

Use 'et format' in CI. Check formatting of all files in CI

6fae7cc

zanderso force-pushed the fix-format-script branch from 6f96d30 to 6fae7cc Compare February 21, 2024 17:02

loic-sharma reviewed Feb 21, 2024

View reviewed changes

loic-sharma approved these changes Feb 21, 2024

View reviewed changes

zanderso merged commit 69ebe43 into flutter:main Feb 21, 2024

zanderso deleted the fix-format-script branch February 21, 2024 17:38

engine-flutter-autoroll mentioned this pull request Feb 21, 2024

Roll Flutter Engine from 52ffcaadea41 to bf5c003085fd (12 revisions) flutter/flutter#143875

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2024

69ebe43a8 Use 'et format' in CI. Check formatting of all files in CI (f…

3d356e9

…lutter/engine#50810)

loic-sharma mentioned this pull request Feb 28, 2024

Add et run command #51037

Merged

	VALIDATION_LOG << "Could not resolve " << "glGetError";
	VALIDATION_LOG << "Could not resolve glGetError";

	<< ")" << " encountered on call to " << name;
	<< ") encountered on call to " << name;

	<< "B " << a.physical_view_inset_left << "L] " << "Gesture Insets: ["
	<< "B " << a.physical_view_inset_left << "L] Gesture Insets: ["

	<< "Profile configurations. " << "For more information, see "
	<< "Profile configurations. For more information, see "

	return stream << "{Pixel:" << " r:" << static_cast<unsigned int>(pixel.red)
	return stream << "{Pixel: r:" << static_cast<unsigned int>(pixel.red)

Use 'et format' in CI. Check formatting of all files in CI #50810

Use 'et format' in CI. Check formatting of all files in CI #50810

Uh oh!

Conversation

zanderso commented Feb 21, 2024

Uh oh!

johnmccutchan left a comment

Choose a reason for hiding this comment

Uh oh!

loic-sharma Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

loic-sharma Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

loic-sharma Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

loic-sharma Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

loic-sharma Feb 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loic-sharma Feb 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loic-sharma Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

loic-sharma Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

loic-sharma Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

loic-sharma left a comment

Choose a reason for hiding this comment

Uh oh!

zanderso commented Feb 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

loic-sharma Feb 21, 2024 •

edited

Loading

loic-sharma Feb 21, 2024 •

edited

Loading