Skip to content

Stop using package:stack_trace and Chain.capture - #8803

Merged
johnmccutchan merged 1 commit into
flutter:masterfrom
johnmccutchan:stack_traces
Mar 15, 2017
Merged

johnmccutchan merged 1 commit into
flutter:masterfrom
johnmccutchan:stack_traces

Conversation

@johnmccutchan

Copy link
Copy Markdown
Contributor
  • Don't use package:stack_trace.
  • Don't use Chain.capture.
  • Fix an instance of aggressive catching of exceptions

Related #8742

@tvolkert

@tvolkert

Copy link
Copy Markdown
Contributor

Looks like this fails on Windows

@johnmccutchan

Copy link
Copy Markdown
Contributor Author

@goderbauer PTAL at the Windows specific fix.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/cc @yjbanov who had occasion to change this code to work this way just last week.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually, n/m -- he didn't add the Chain.capture(), and this is still semantically the same

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's a slight semantic difference here -- previously, it would use the value of traceStartup that was passed to its constructor if it was unspecified in run(). Now it'll always default to false

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@tvolkert

Copy link
Copy Markdown
Contributor

LGTM with 1 comment

- [x] Don't use package:stack_trace.
- [x] Don't use Chain.capture.
- [x] Fix an instance of aggressive catching of exceptions

Related #8742

@yjbanov yjbanov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we also remove the dependency in pubspec.yaml?

@johnmccutchan
johnmccutchan merged commit 4c91b6e into flutter:master Mar 15, 2017
@Hixie

Hixie commented Mar 16, 2017

Copy link
Copy Markdown
Contributor

+1 to this patch

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants