Remove "note that" in our documentation (as per style guide) #120842

Hixie · 2023-02-15T23:26:24Z

Documentation improvements
Remove "Note:" and "Note that" as per style guide

christopherfujino

LGTM (at least tool code)

loic-sharma · 2023-02-17T19:11:13Z

dev/conductor/core/lib/src/repository.dart

@@ -129,8 +129,7 @@ abstract class Repository {
          'Fetch ${upstreamRemote.name} to ensure we have latest refs',
          workingDirectory: _checkoutDirectory!.path,
        );
-        // Note: if [initialRef] is a remote ref the checkout will be left in a
-        // detached HEAD state.
+        // If [initialRef] is a remote ref the checkout will be left in a detached HEAD state.


Suggested change

// If [initialRef] is a remote ref the checkout will be left in a detached HEAD state.

// If [initialRef] is a remote ref, the checkout will be left in a detached HEAD state.

loic-sharma · 2023-02-17T19:16:23Z

dev/tools/gen_keycodes/lib/physical_key_data.dart

  ///
  /// We parse the commented out lines as well as the non-commented lines, so
  /// that we can get names for all of the available scan codes, not just ones
  /// defined for the generic profile.
  ///
-  /// Also, note that some keys (notably MEDIA_EJECT) can be mapped to more than
+  /// Some keys (notably MEDIA_EJECT) can be mapped to more than


Suggested change

/// Some keys (notably MEDIA_EJECT) can be mapped to more than

/// Some keys (notably `MEDIA_EJECT`) can be mapped to more than

loic-sharma · 2023-02-17T19:22:15Z

packages/flutter/lib/src/material/dialog.dart

-  /// arranged into a column. This parameter provides additional
-  /// vertical space in between buttons when it does overflow.
+  /// arranged into a column. This parameter provides additional vertical space
+  /// in between buttons when it does overflow.


Suggested change

/// in between buttons when it does overflow.

/// between buttons when it does overflow.

loic-sharma · 2023-02-17T19:27:22Z

packages/flutter/lib/src/material/expansion_panel.dart

+/// [ExpansionPanel.isExpanded]. For [ExpansionPanelList.radio] widgets, the
+/// callback is invoked both for the previously open panel, which is closing,
+/// and the previously closed panel, which is opening, and the widget tracks the
+/// open state internally.


The last "and the widget tracks the open state internally" statement feels disjointed to me. Consider reordering:

Suggested change

/// [ExpansionPanel.isExpanded]. For [ExpansionPanelList.radio] widgets, the

/// callback is invoked both for the previously open panel, which is closing,

/// and the previously closed panel, which is opening, and the widget tracks the

/// open state internally.

/// [ExpansionPanel.isExpanded]. For [ExpansionPanelList.radio] widgets, the

/// open state is tracked internally and the callback is invoked

/// both for the previously open panel, which is closing, and the

// previously closed panel, which is opening.

(This suggestion's formatting is likely off)

loic-sharma · 2023-02-17T19:34:09Z

packages/flutter/lib/src/material/expansion_tile.dart

+  /// be the same as the widest child widget in [children]. It is not
+  /// necessarily the width of [Column] is equal to the width of expanded tile.


Consider something like:

Suggested change

/// be the same as the widest child widget in [children]. It is not

/// necessarily the width of [Column] is equal to the width of expanded tile.

/// be the same as the widest child widget in [children]. The width of

/// [Column] might not be equal to the width of the expanded tile.

loic-sharma · 2023-02-17T19:36:18Z

packages/flutter/lib/src/material/text_theme.dart

-  /// styles will be deprecated and removed eventually.
+  /// The 2018 styles cannot be mixed with the 2021 styles. Only one or the
+  /// other is allowed in this constructor. The 2018 styles are deprecated and
+  /// will eventually be removed.


The American school system is failing me here, but the adverb should be after the verb right?

Suggested change

/// will eventually be removed.

/// will be removed eventually.

either is fine as far as i'm aware

Ah gotcha, feel free to ignore then :)

loic-sharma · 2023-02-17T19:37:34Z

packages/flutter/lib/src/material/theme_data.dart

-  /// not change any of these properties in the resulting [ThemeData].
+  ///
+  /// If a [ThemeData] is _constructed_ with [useMaterial3] set to true, then
+  /// some properties will get updated defaults. The [ThemeData.copyWith] method


This is an unexpected call out, consider adding However:

Suggested change

/// some properties will get updated defaults. The [ThemeData.copyWith] method

/// some properties will get updated defaults. However, the [ThemeData.copyWith] method

loic-sharma · 2023-02-17T19:42:52Z

packages/flutter_tools/lib/src/commands/build_aar.dart

-      'https://flutter.dev/go/build-aar\n'
-      'Note: this command builds applications assuming that the entrypoint is lib/main.dart. '
+      'To learn more about how to use these artifacts, see https://flutter.dev/go/build-aar\n'
+      'This command builds applications assuming that the entrypoint is lib/main.dart. '
      'This cannot currently be configured.';


Suggested change

'This cannot currently be configured.';

'This cannot be configured currently.';

This is one of those cases where the earlier adverb acts as emphasis, I think.

Feel free to ignore this, I don't feel strongly :)

loic-sharma · 2023-02-17T19:43:44Z

packages/flutter_tools/lib/src/commands/build_aar.dart

-      'https://flutter.dev/go/build-aar\n'
-      'Note: this command builds applications assuming that the entrypoint is lib/main.dart. '
+      'To learn more about how to use these artifacts, see https://flutter.dev/go/build-aar\n'
+      'This command builds applications assuming that the entrypoint is lib/main.dart. '


Consider active voice here:

Suggested change

'This command builds applications assuming that the entrypoint is lib/main.dart. '

'This command assumes that the entrypoint is lib/main.dart. '

I don't feel strongly though.

loic-sharma

Nice! 🎉

Hixie · 2023-02-17T20:57:13Z

Applied your fixes (and a couple others I noticed while doing so), modulo the adverb/verb ones per the comments above.

Hixie · 2023-02-17T20:57:51Z

thanks for the reviews, will land on green

flutter/flutter#120842)

* df98689c9 2be7253c9 Roll Fuchsia Linux SDK from q7u2WyX2BSRBIzyTW... to yT4JLKTCWWwbRwB0l... (flutter/engine#39679) (flutter/flutter#120898) * cacef57b6 [flutter_tools] Skip over "Resolving dependencies..." text in integration tests (flutter/flutter#120077) * 34102ca3b Migrate channels to pkg:integration _test (flutter/flutter#120833) * df13ea248 Roll Flutter Engine from 2be7253c9b10 to e4cb80e22ee1 (2 revisions) (flutter/flutter#120903) * a2e65f7c3 Roll Flutter Engine from e4cb80e22ee1 to 4a90fbcd6901 (2 revisions) (flutter/flutter#120911) * e00241a06 Enable Windows plugin tests (flutter/flutter#119345) * 09ad9f3cd Document ScrollPhysics invariant requiring ballistic motion (flutter/flutter#120400) * 6029de2fb Update switch template (flutter/flutter#120919) * 229d70ea3 Roll Flutter Engine from 4a90fbcd6901 to bddfc1c4dcaa (5 revisions) (flutter/flutter#120920) * 206c6ae99 roll packages (flutter/flutter#120922) * 9fcaaebb5 Roll Flutter Engine from bddfc1c4dcaa to 6602fc753525 (3 revisions) (flutter/flutter#120928) * 00c0a07fa Increase Linux docs_test timeout (flutter/flutter#120899) * e29a79975 946b29198 [dart:ui] Introduce `PlatformDispatcher.implicitView` (flutter/engine#39553) (flutter/flutter#120939) * 081cd5776 650db7a72 [macOS] Eliminate mirrors support (flutter/engine#39694) (flutter/flutter#120943) * 875e48c69 52a4fb4c5 Roll Skia from b1800a8b9595 to d0df677ffd5e (13 revisions) (flutter/engine#39699) (flutter/flutter#120947) * 78d058f46 6e92c0c28 Roll Fuchsia Mac SDK from xl9Y8o-9FDyvPogki... to haDvcC5VzWVdQs9Rs... (flutter/engine#39700) (flutter/flutter#120950) * 298d8c76b Revert "Remove references to Observatory (#118577)" (flutter/flutter#120929) * 674254c03 Always use the testbed in web_test.dart so `environment` is populated. (flutter/flutter#120984) * c4d40cc15 Modify the updateChildren method deep copy _children (flutter/flutter#120773) * 9367641ce clean up (flutter/flutter#120934) * 51712b90a Roll Plugins from d699b4a to 8f3419b (7 revisions) (flutter/flutter#120993) * c3587c62e Add `InheritedTheme` support to `ScrollbarTheme` (flutter/flutter#120970) * 08b409ab0 Roll Flutter Engine from 6e92c0c28410 to bd37a3992b50 (16 revisions) (flutter/flutter#121004) * f78513685 [web] Temporarily disable a line boundary test (flutter/flutter#121005) * 9fe556705 Print sub process that failed to run in tool (flutter/flutter#120999) * 6205c110d Remove "note that" in our documentation (as per style guide) (flutter/flutter#120842) * 1daa0be4f Fix scrollable to clear inner semantics node if it does not use two p… (flutter/flutter#120996) * 7f19b7485 0a27673d7 Roll Skia from 02890036028e to 0e444e355607 (9 revisions) (flutter/engine#39723) (flutter/flutter#121008) * 48d2dfc72 e7fde3f72 [web] Make glassPaneElement and glassPaneShadow non-nullable (flutter/engine#39692) (flutter/flutter#121009) * 610450523 2b2780185 Roll Skia from 0e444e355607 to 4b79e398dfe0 (5 revisions) (flutter/engine#39725) (flutter/flutter#121016) * f99f47280 Remove the deprecated accentColor from ThemeData (flutter/flutter#120932) * 2b4c96088 Remove more references to dart:ui.window (flutter/flutter#120994) * 0fa652752 Roll Flutter Engine from 2b2780185dd5 to a37e27b77008 (2 revisions) (flutter/flutter#121020) * 9281114fb Roll Flutter Engine from a37e27b77008 to 2fdce9a96367 (2 revisions) (flutter/flutter#121023) * 4dd555d32 Roll Flutter Engine from 2fdce9a96367 to 9a3c3e462fce (3 revisions) (flutter/flutter#121025) * 66dce657f Roll Flutter Engine from 9a3c3e462fce to 3777ed51774f (2 revisions) (flutter/flutter#121029) * a5b53a6d2 a9db42c3e Roll Skia from 733a19f6a625 to 2f05923f825e (3 revisions) (flutter/engine#39744) (flutter/flutter#121030) * 0be7c3f30 Roll Flutter Engine from a9db42c3edc2 to c22c64812243 (2 revisions) (flutter/flutter#121041)

flutter/flutter#120842)

* 674254c03 Always use the testbed in web_test.dart so `environment` is populated. (flutter/flutter#120984) * c4d40cc15 Modify the updateChildren method deep copy _children (flutter/flutter#120773) * 9367641ce clean up (flutter/flutter#120934) * 51712b90a Roll Plugins from d699b4a to 8f3419b (7 revisions) (flutter/flutter#120993) * c3587c62e Add `InheritedTheme` support to `ScrollbarTheme` (flutter/flutter#120970) * 08b409ab0 Roll Flutter Engine from 6e92c0c28410 to bd37a3992b50 (16 revisions) (flutter/flutter#121004) * f78513685 [web] Temporarily disable a line boundary test (flutter/flutter#121005) * 9fe556705 Print sub process that failed to run in tool (flutter/flutter#120999) * 6205c110d Remove "note that" in our documentation (as per style guide) (flutter/flutter#120842) * 1daa0be4f Fix scrollable to clear inner semantics node if it does not use two p… (flutter/flutter#120996) * 7f19b7485 0a27673d7 Roll Skia from 02890036028e to 0e444e355607 (9 revisions) (flutter/engine#39723) (flutter/flutter#121008) * 48d2dfc72 e7fde3f72 [web] Make glassPaneElement and glassPaneShadow non-nullable (flutter/engine#39692) (flutter/flutter#121009) * 610450523 2b2780185 Roll Skia from 0e444e355607 to 4b79e398dfe0 (5 revisions) (flutter/engine#39725) (flutter/flutter#121016) * f99f47280 Remove the deprecated accentColor from ThemeData (flutter/flutter#120932) * 2b4c96088 Remove more references to dart:ui.window (flutter/flutter#120994) * 0fa652752 Roll Flutter Engine from 2b2780185dd5 to a37e27b77008 (2 revisions) (flutter/flutter#121020) * 9281114fb Roll Flutter Engine from a37e27b77008 to 2fdce9a96367 (2 revisions) (flutter/flutter#121023) * 4dd555d32 Roll Flutter Engine from 2fdce9a96367 to 9a3c3e462fce (3 revisions) (flutter/flutter#121025) * 66dce657f Roll Flutter Engine from 9a3c3e462fce to 3777ed51774f (2 revisions) (flutter/flutter#121029) * a5b53a6d2 a9db42c3e Roll Skia from 733a19f6a625 to 2f05923f825e (3 revisions) (flutter/engine#39744) (flutter/flutter#121030) * 0be7c3f30 Roll Flutter Engine from a9db42c3edc2 to c22c64812243 (2 revisions) (flutter/flutter#121041)

flutter/flutter#120842)

Hixie force-pushed the minor_tweaks branch 3 times, most recently from edd4f7c to 3d66dfe Compare February 16, 2023 00:39

Hixie changed the title ~~Minor tweaks~~ Remove "note that" in our documentation (as per style guide) Feb 16, 2023

Hixie marked this pull request as ready for review February 16, 2023 01:27

Hixie requested a review from christopherfujino as a code owner February 16, 2023 01:27

christopherfujino approved these changes Feb 16, 2023

View reviewed changes

loic-sharma reviewed Feb 17, 2023

View reviewed changes

loic-sharma approved these changes Feb 17, 2023

View reviewed changes

Hixie added 3 commits February 17, 2023 12:49

lerp documentation

e573b20

Remove Note, Note That from repo

eb784d6

Improve BorderSide documentation.

2eb0a89

apply review comments

98b8fe7

Hixie force-pushed the minor_tweaks branch from 3d66dfe to 98b8fe7 Compare February 17, 2023 20:57

Hixie added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 17, 2023

auto-submit bot merged commit 6205c11 into flutter:master Feb 17, 2023

This was referenced Feb 18, 2023

Roll Flutter from 170539f83dc8 to 0be7c3f30d64 (38 revisions) flutter/packages#3225

Merged

Roll Flutter from 298d8c76ba78 to 0be7c3f30d64 (21 revisions) flutter/plugins#7193

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 18, 2023

6205c110d Remove "note that" in our documentation (as per style guide) (

564b595

flutter/flutter#120842)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 18, 2023

6205c110d Remove "note that" in our documentation (as per style guide) (

e9fad07

flutter/flutter#120842)

engine-flutter-autoroll mentioned this pull request Feb 18, 2023

Roll Flutter from 298d8c76ba78 to 0be7c3f30d64 (21 revisions) flutter/plugins#7194

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 18, 2023

6205c110d Remove "note that" in our documentation (as per style guide) (

dbf3ad4

flutter/flutter#120842)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

6205c110d Remove "note that" in our documentation (as per style guide) (

93b6c67

flutter/flutter#120842)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

6205c110d Remove "note that" in our documentation (as per style guide) (

d78d143

flutter/flutter#120842)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove "note that" in our documentation (as per style guide) #120842

Remove "note that" in our documentation (as per style guide) #120842

Hixie commented Feb 15, 2023

christopherfujino left a comment

loic-sharma Feb 17, 2023

loic-sharma Feb 17, 2023

loic-sharma Feb 17, 2023

loic-sharma Feb 17, 2023

loic-sharma Feb 17, 2023

loic-sharma Feb 17, 2023 •

edited

Hixie Feb 17, 2023

loic-sharma Feb 17, 2023

loic-sharma Feb 17, 2023

loic-sharma Feb 17, 2023

Hixie Feb 17, 2023

loic-sharma Feb 17, 2023

loic-sharma Feb 17, 2023

loic-sharma left a comment

Hixie commented Feb 17, 2023

Hixie commented Feb 17, 2023

	// If [initialRef] is a remote ref the checkout will be left in a detached HEAD state.
	// If [initialRef] is a remote ref, the checkout will be left in a detached HEAD state.

	/// Some keys (notably MEDIA_EJECT) can be mapped to more than
	/// Some keys (notably `MEDIA_EJECT`) can be mapped to more than

	/// in between buttons when it does overflow.
	/// between buttons when it does overflow.

		/// be the same as the widest child widget in [children]. It is not
		/// necessarily the width of [Column] is equal to the width of expanded tile.

	/// will eventually be removed.
	/// will be removed eventually.

	/// some properties will get updated defaults. The [ThemeData.copyWith] method
	/// some properties will get updated defaults. However, the [ThemeData.copyWith] method

	'This cannot currently be configured.';
	'This cannot be configured currently.';

	'This command builds applications assuming that the entrypoint is lib/main.dart. '
	'This command assumes that the entrypoint is lib/main.dart. '

Remove "note that" in our documentation (as per style guide) #120842

Remove "note that" in our documentation (as per style guide) #120842

Conversation

Hixie commented Feb 15, 2023

christopherfujino left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loic-sharma Feb 17, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loic-sharma left a comment

Choose a reason for hiding this comment

Hixie commented Feb 17, 2023

Hixie commented Feb 17, 2023

loic-sharma Feb 17, 2023 •

edited