Fixes for Style Guide for Flutter Repo #150167
Conversation
github-actions
bot
added
c: contributor-productivity
|
My merge commit was unsigned, initially. I signed it and force-pushed the new commit. |
There was a problem hiding this comment.
Now that people are much more likely to have this wiki page open in any number of different editors, it makes sense to group the paragraphs together (rather than having line breaks every 80 characters in some places, and no line breaks in others).
Overall I'm a huge fan of the reformatting & grammar fixing here—just had a few questions before we merge.
|
@nate-thegrate, I pushed the changes you requested. Thanks for reviewing this PR so quickly. This is my first Flutter PR. What's the proper etiquette for "Resolve conversation" button? Do I wait for the reviewer to press it? |
No problem! As far as whether to click the "Resolve conversation" button, it isn't mentioned anywhere in the wiki's code review section. I think it makes sense to resolve a comment in your PR if it's been addressed or is no longer relevant—that way, the next person to review won't waste any time reading it. |
nate-thegrate
left a comment
nate-thegrate
left a comment
There was a problem hiding this comment.
LGTM! 👍
Thanks for making this contribution!
gnprice
left a comment
gnprice
left a comment
There was a problem hiding this comment.
A large-scale reformatting like this should never be mixed with non-formatting changes, because it makes it impossible to effectively review the changes.
So if you'd like to pursue unwrapping all the paragraphs onto single giant lines, that should be its own PR. (Personally I think that's an unhelpful direction — I'd much prefer to move toward semantic line breaks, the same convention used in the other large Markdown files in the Flutter project. But others may have different reactions.)
Then the edits changing sentence case to title case, changing ### to #, splitting one item into two, changing "Designing an API" to "API design", and all the rest can be a separate PR. That way people can see each of those changes in order to review them.
|
@gnprice, thanks for the recommendation. @nate-thegrate suggested the same on Discord. I force-pushed a single commit for this PR with just the typo, grammar, and markdown changes. That was easier than reverting earlier commits, as I had made the whitespace changes first. |
nate-thegrate
left a comment
nate-thegrate
left a comment
There was a problem hiding this comment.
I have to say: it's much, much easier to read the diffs this time around.
Thanks for putting in the work and for being receptive to feedback 👍
gnprice
left a comment
gnprice
left a comment
There was a problem hiding this comment.
Thanks, this diff is a lot more readable.
There are some typo and markup fixes here which are good. I think the stylistic changes in this PR generally aren't helpful, though. I'd be glad to see a revised version that focused just on the changes that are clear improvements.
nate-thegrate
left a comment
nate-thegrate
left a comment
There was a problem hiding this comment.
Sending a 3rd consecutive approval 👍
|
Thanks, everyone, for the attention on this PR. I have reverted the edits and retained only the grammar and syntax changes. My impression reading this document is that it needs some love from a writer, but maybe that can happen in the future. I'm not a writer, anyway. |
Thanks, @nate-thegrate, for the support on this PR. |
gnprice
added
the
autosubmit
Manual roll Flutter from e726eb4 to 15f95ce (48 revisions) Manual roll requested by [email protected] flutter/flutter@e726eb4...15f95ce 2024-06-28 [email protected] Roll Flutter Engine from ddd4814b9d40 to 94591ffb20df (5 revisions) (flutter/flutter#150968) 2024-06-27 [email protected] Manual engine roll to ddd4814 (flutter/flutter#150952) 2024-06-27 [email protected] local lint copy gradle task config (flutter/flutter#150957) 2024-06-27 [email protected] Roll Flutter Engine from b42c80460538 to d1506c12808e (3 revisions) (flutter/flutter#150951) 2024-06-27 [email protected] [tool] make the `systemTempDirectory` getter on `ErrorHandlingFileSystem` wrap the underlying filesystem's temp directory in a`ErrorHandlingDirectory` (flutter/flutter#150876) 2024-06-27 [email protected] Have flutter.js load local canvaskit instead of the CDN when appropriate (flutter/flutter#150806) 2024-06-27 [email protected] Roll Flutter Engine from a9194f0f01f4 to b42c80460538 (10 revisions) (flutter/flutter#150940) 2024-06-27 [email protected] [a11y] Reland [#149375 ] Update semantics in dropdown.dart (flutter/flutter#150578) 2024-06-27 [email protected] Bump dartdoc to 8.0.9+1 (flutter/flutter#150935) 2024-06-27 [email protected] add onFocus to text fields (flutter/flutter#150648) 2024-06-27 [email protected] Fixes `flutter build ipa` failure: Command line name "app-store" is deprecated. Use "app-store-connect" (flutter/flutter#150407) 2024-06-27 [email protected] Copy any previous `IconThemeData` instead of overwriting it in CupertinoButton (flutter/flutter#149777) 2024-06-27 [email protected] Improve the behavior of scrollbar drag-scrolls triggered by the trackpad (flutter/flutter#150275) 2024-06-27 [email protected] feat: Add autofocus for `MenuItemButton` (flutter/flutter#139396) 2024-06-27 [email protected] Roll Flutter Engine from 1d5e3cc55a75 to a9194f0f01f4 (7 revisions) (flutter/flutter#150888) 2024-06-26 [email protected] Reland "Remove dual_screen from new_gallery integration test" (flutter/flutter#150873) 2024-06-26 [email protected] Switch to more reliable flutter.dev link destinations in the tool (flutter/flutter#150587) 2024-06-26 [email protected] Adding `@docImport`s to the `animation` library (flutter/flutter#150798) 2024-06-26 [email protected] Remove CODEOWNERS trailing whitespace (flutter/flutter#150882) 2024-06-26 [email protected] Roll Flutter Engine from e03cf86c4170 to 1d5e3cc55a75 (3 revisions) (flutter/flutter#150875) 2024-06-26 [email protected] Remind folks we are moving. (flutter/flutter#150872) 2024-06-26 [email protected] Remove `bringup: true` from web test shard. (flutter/flutter#150785) 2024-06-26 [email protected] Roll Flutter Engine from c0017bed42c2 to e03cf86c4170 (1 revision) (flutter/flutter#150867) 2024-06-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Remove `dual_screen` from `new_gallery` integration test (#150808)" (flutter/flutter#150871) 2024-06-26 [email protected] Remove `dual_screen` from `new_gallery` integration test (flutter/flutter#150808) 2024-06-26 [email protected] Roll Flutter Engine from d4624a36712b to c0017bed42c2 (4 revisions) (flutter/flutter#150865) 2024-06-26 [email protected] Fixes for Style Guide for Flutter Repo (flutter/flutter#150167) 2024-06-26 [email protected] Roll Flutter Engine from da62c629ed5c to d4624a36712b (3 revisions) (flutter/flutter#150852) 2024-06-26 [email protected] Use `Isolate.packageConfigSync! to locate the packageconfig of flutter tools (flutter/flutter#150340) 2024-06-26 [email protected] Roll Flutter Engine from 25af762ffbb3 to da62c629ed5c (2 revisions) (flutter/flutter#150829) 2024-06-26 [email protected] Fix leaky tests. (flutter/flutter#150817) 2024-06-26 [email protected] Roll Flutter Engine from 94023d711db3 to 25af762ffbb3 (4 revisions) (flutter/flutter#150818) 2024-06-26 [email protected] Roll pub packages (flutter/flutter#150810) 2024-06-25 [email protected] Remove reference to `MaterialApp` and `showCupertinoModalPopup` from `CupertinoAlertDialog` (flutter/flutter#150725) 2024-06-25 [email protected] Read `AndroidManifest.xml` and emit `manifest-impeller-(enabled|disabled)` analytics (flutter/flutter#150791) 2024-06-25 [email protected] [flutter_tools] Shut down Chromium cleanly using a command sent through the debug protocol (flutter/flutter#150645) 2024-06-25 [email protected] Reland fix inputDecorator hint color on M3 (flutter/flutter#150278) 2024-06-25 [email protected] Roll Flutter Engine from 62e0b5f9c340 to 94023d711db3 (7 revisions) (flutter/flutter#150797) 2024-06-25 [email protected] Fix collapsed InputDecorator minimum height (flutter/flutter#150770) 2024-06-25 [email protected] Add more warm up frame docs (flutter/flutter#150464) 2024-06-25 [email protected] Roll pub packages (flutter/flutter#150739) 2024-06-25 [email protected] Add `focusNode`, `focusColor`, `onFocusChange`, `autofocus` to `CupertinoButton` (flutter/flutter#150721) 2024-06-25 [email protected] Document RenderObject._relayoutBoundary and its invariant; small refactors (flutter/flutter#150527) 2024-06-25 [email protected] Roll Flutter Engine from 6313b1e5afd7 to 62e0b5f9c340 (1 revision) (flutter/flutter#150790) ...
6 participants
Fix broken links, typos, and inconsistent formatting in the Style Guide for Flutter Repo.
Fixes 150164.
Pre-launch Checklist
///).