Skip to content
This repository has been archived by the owner on Oct 17, 2024. It is now read-only.

Resolve null safety migration TODOS #135

Merged
merged 1 commit into from
Aug 26, 2020
Merged

Resolve null safety migration TODOS #135

merged 1 commit into from
Aug 26, 2020

Conversation

natebosch
Copy link
Contributor

  • Switch from StackTrace.fromString to AsyncError.defaultStackTrace.
  • Remove a TODO about non-nullable StackTrace, by the time we merged
    null safety the type was already non-nullable and the TODO was stale.

Style tweak - remove the argument names from a Function type that are
redundant against their type names.

- Switch from `StackTrace.fromString` to `AsyncError.defaultStackTrace`.
- Remove a TODO about non-nullable `StackTrace`, by the time we merged
  null safety the type was already non-nullable and the TODO was stale.

Style tweak - remove the argument names from a Function type that are
redundant against their type names.
// the sdk.
typedef HandleError<T> = void Function(
Object error, StackTrace stackTrace, EventSink<T> sink);
typedef HandleError<T> = void Function(Object error, StackTrace, EventSink<T>);
Copy link
Contributor

Choose a reason for hiding this comment

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

it feels weird to have the name specified for one of these and not the others, but 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the only one where the type doesn't tell you what it is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ya, I get that, but it still feels weird to me. I would be fine with removing it, as the name of the typedef itself I think makes it pretty clear, or I would also be fine with it as you have it.

@natebosch natebosch merged commit cccc8aa into master Aug 26, 2020
@natebosch natebosch deleted the resolve-todos branch August 26, 2020 17:01
mosuem pushed a commit to dart-lang/core that referenced this pull request Oct 14, 2024
- Switch from `StackTrace.fromString` to `AsyncError.defaultStackTrace`.
- Remove a TODO about non-nullable `StackTrace`, by the time we merged
  null safety the type was already non-nullable and the TODO was stale.

Style tweak - remove the argument names from a Function type that are
redundant against their type names.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

3 participants