Skip to content

Remove unused code from tests #104550

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.

Sign up for GitHub

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

Jump to bottom
Merged
fluttergithubbot merged 2 commits into from
May 25, 2022
Merged

Remove unused code from tests #104550

fluttergithubbot merged 2 commits into from
May 25, 2022

Conversation

@goderbauer
Copy link
Member

@goderbauer goderbauer commented May 24, 2022

The dead code was found by running dart run dart_code_metrics:metrics check-unused-code test from https://pub.dev/packages/dart_code_metrics in packages/flutter.

Other notes about the tool:

  • The tool didn't find anything interesting in non-test code of the framework.
  • There were a few false positives in the framework and in test code.

@flutter-dashboard flutter-dashboard bot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: text input Entering text in a text field or keyboard related problems f: gestures flutter/packages/flutter/gestures repository. f: material design flutter/packages/flutter/material repository. f: routes Navigator, Router, and related APIs. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. labels May 24, 2022
@goderbauer
Copy link
Member Author

goderbauer commented May 24, 2022

Regarding the dead code this tool found in tests: Ideally, we'd want to avoid accumulating this and there are a couple of lint proposals that would help: dart-lang/sdk#57824, dart-lang/sdk#57580, dart-lang/sdk#58292. I believe, any of these would have avoided the dead code from making (or remaining) in the repository. (cc @pq)

@goderbauer goderbauer requested a review from gspencergoog May 24, 2022 22:13
This was referenced May 24, 2022
Closed
Closed
Closed
gspencergoog
gspencergoog approved these changes May 24, 2022
View reviewed changes
Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

Excellent!

This was referenced May 24, 2022
Merged
Merged
@pq
Copy link
Contributor

pq commented May 25, 2022

This is pretty amazing. Nice. Thanks for sharing. And cheers to the dart_code_metrics folks! 🍻

@fluttergithubbot fluttergithubbot merged commit d5fbc37 into flutter:master May 25, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 25, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 25, 2022
@engine-flutter-autoroll
d5fbc37 Remove unused code from tests (flutter/flutter#104550)
7bfc572
@goderbauer goderbauer deleted the test-cleanup branch May 25, 2022 16:55
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 25, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 25, 2022
@engine-flutter-autoroll
d5fbc37 Remove unused code from tests (flutter/flutter#104550)
3d4549e
@incendial
Copy link

incendial commented Jul 12, 2022

@goderbauer hi, co-author of Dart Code Metrics here 🙂. So happy to see that the tool helped find and remove unused code! Could you please share more details about the false-positives part? If you don't think this PR is a great place to discuss it, feel free to open an issue here https://github.com/dart-code-checker/dart-code-metrics/issues/new/choose

This is pretty amazing. Nice. Thanks for sharing. And cheers to the dart_code_metrics folks! 🍻

Thank you! ❤️

@marandaneto marandaneto mentioned this pull request Jul 19, 2022
Closed
camsim99 pushed a commit to camsim99/flutter that referenced this pull request Aug 10, 2022
@goderbauer @camsim99
Remove unused code from tests (flutter#104550)
4071efe
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022
@engine-flutter-autoroll
d5fbc37 Remove unused code from tests (flutter/flutter#104550)
432dc62
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 30, 2022
@engine-flutter-autoroll
d5fbc37 Remove unused code from tests (flutter/flutter#104550)
a39f654
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gspencergoog gspencergoog gspencergoog approved these changes

Assignees

No one assigned

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: text input Entering text in a text field or keyboard related problems f: gestures flutter/packages/flutter/gestures repository. f: material design flutter/packages/flutter/material repository. f: routes Navigator, Router, and related APIs. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@goderbauer @pq @incendial @gspencergoog @fluttergithubbot