Skip to content

[flutter_tool] Report to analytics when the tool is killed by a signal #41493

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
zanderso merged 2 commits into from
Sep 30, 2019
Merged

[flutter_tool] Report to analytics when the tool is killed by a signal #41493

zanderso merged 2 commits into from
Sep 30, 2019

Conversation

@zanderso
Copy link
Member

@zanderso zanderso commented Sep 27, 2019

Description

This PR adds signal handlers such that the tool will report a command result event and command timing when the tool is killed by a signal (SIGINT, SIGTERM).

Tests

I added the following tests:

Tests in signals_test.dart and flutter_command_test.dart.

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.

@zanderso zanderso added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 27, 2019
jonahwilliams
jonahwilliams approved these changes Sep 27, 2019
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

christopherfujino
christopherfujino approved these changes Sep 27, 2019
View reviewed changes
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

lgtm

christopherfujino
christopherfujino suggested changes Sep 27, 2019
View reviewed changes
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

looks like CI is failing

@codecov
Copy link

codecov bot commented Sep 27, 2019 •
edited
Loading

Codecov Report

Merging #41493 into master will decrease coverage by 1.25%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #41493      +/-   ##
==========================================
- Coverage   60.67%   59.41%   -1.26%     
==========================================
  Files         193      193              
  Lines       18833    18841       +8     
==========================================
- Hits        11426    11194     -232     
- Misses       7407     7647     +240
Flag Coverage Δ
#flutter_tool 59.41% <92.85%> (-1.26%) ⬇️
Impacted Files Coverage Δ
.../flutter_tools/lib/src/runner/flutter_command.dart 83.4% <100%> (+0.64%) ⬆️
packages/flutter_tools/lib/src/base/signals.dart 93.54% <80%> (+3.54%) ⬆️
...tter_tools/lib/src/build_system/targets/macos.dart 2.42% <0%> (-46.07%) ⬇️
packages/flutter_tools/lib/src/base/flags.dart 5.55% <0%> (-44.45%) ⬇️
...ckages/flutter_tools/lib/src/base/fingerprint.dart 54.66% <0%> (-40%) ⬇️
packages/flutter_tools/lib/src/cache.dart 23.88% <0%> (-22.1%) ⬇️
.../flutter_tools/lib/src/android/android_device.dart 27.44% <0%> (-5.82%) ⬇️
packages/flutter_tools/lib/src/template.dart 95.23% <0%> (-4.77%) ⬇️
packages/flutter_tools/lib/src/context_runner.dart 70.17% <0%> (-1.76%) ⬇️
packages/flutter_tools/lib/src/version.dart 91.9% <0%> (-1.43%) ⬇️
... 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 feaaa5d...dc1f596. Read the comment docs.

@zanderso
Copy link
Member Author

zanderso commented Sep 27, 2019

Fixed tests.

@zanderso zanderso merged commit 45f3c8d into flutter:master Sep 30, 2019
@zanderso zanderso deleted the command-result-killed branch September 30, 2019 14:38
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019
@zanderso @Inconnu08
[flutter_tool] Report to analytics when the tool is killed by a signal (
20c02d0 
flutter#41493)

* [flutter_tool] Report to analytics when the tool is killed by a signal

* Fix analyzer lint
@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.

Reviewers

2 more reviewers

@christopherfujino christopherfujino christopherfujino requested changes

@jonahwilliams jonahwilliams jonahwilliams approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@zanderso @christopherfujino @jonahwilliams @googlebot