Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bad indentations(mainly around collection literals) #41355

Merged
merged 1 commit into from
Sep 27, 2019

Conversation

a14n
Copy link
Contributor

@a14n a14n commented Sep 26, 2019

Description

fix some bad indentations (mainly around collection literals) in packages/flutter/ and packages/flutter_tools/

Related Issues

None

Tests

None

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (Please read Handling breaking changes). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.

@fluttergithubbot fluttergithubbot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels. labels Sep 26, 2019
@a14n a14n added the team Infra upgrades, team productivity, code health, technical debt. See also team: labels. label Sep 26, 2019
@codecov
Copy link

codecov bot commented Sep 26, 2019

Codecov Report

Merging #41355 into master will decrease coverage by 0.85%.
The diff coverage is 56.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #41355      +/-   ##
==========================================
- Coverage   60.45%   59.59%   -0.86%     
==========================================
  Files         192      192              
  Lines       18724    18723       -1     
==========================================
- Hits        11319    11158     -161     
- Misses       7405     7565     +160
Flag Coverage Δ
#flutter_tool 59.59% <56.25%> (-0.86%) ⬇️
Impacted Files Coverage Δ
...utter_tools/lib/src/build_system/targets/dart.dart 50% <ø> (ø) ⬆️
...utter_tools/lib/src/build_runner/build_runner.dart 0% <ø> (ø) ⬆️
packages/flutter_tools/lib/src/bundle.dart 79.34% <ø> (ø) ⬆️
...ackages/flutter_tools/lib/src/commands/unpack.dart 5.05% <ø> (ø) ⬆️
packages/flutter_tools/lib/src/doctor.dart 75.2% <0%> (ø) ⬆️
...utter_tools/lib/src/build_runner/build_script.dart 19.09% <0%> (ø) ⬆️
...utter_tools/lib/src/test/flutter_web_platform.dart 0% <0%> (ø) ⬆️
packages/flutter_tools/lib/src/vmservice.dart 40.89% <100%> (+0.16%) ⬆️
packages/flutter_tools/lib/src/version.dart 91.9% <100%> (-1.43%) ⬇️
packages/flutter_tools/lib/src/android/gradle.dart 46.5% <0%> (-29.31%) ⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e024744...7e82709. Read the comment docs.

@a14n a14n removed the a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) label Sep 26, 2019
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

rect: const Rect.fromLTRB(0.0, 0.0, 48.0, 48.0),
children: <TestSemantics>[],
),
]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should there be a trailing comma here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a rule to lint missing trailing commas but (for now) I made an exception to make it only lint missing trailing commas if the argument list has a length > 1. That's why there's still a missing trailing comma here.

This exception is here to allow things like:

          information.add(ErrorDescription(
            'The test API method "${originalGuarder.methodName}" '
            'from class ${originalGuarder.className} '
            'was called from ${originalGuarder.callerFile} '
            'on line ${originalGuarder.callerLine}, '
            'but never completed before its parent scope closed.'   // there should be a trailing comma here
          ));

From dartfmt POV there should be a trailing comma to keep this kind of formatting. Perhaps/eventually the rule should remove the exception. WDYT?

@a14n a14n merged commit f11c341 into flutter:master Sep 27, 2019
@a14n a14n deleted the indent branch September 27, 2019 08:46
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. team Infra upgrades, team productivity, code health, technical debt. See also team: labels. tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants