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

chore: refactor code style #186

Merged
merged 1 commit into from
Dec 7, 2023
Merged

chore: refactor code style #186

merged 1 commit into from
Dec 7, 2023

Conversation

LucasXu0
Copy link
Contributor

@LucasXu0 LucasXu0 commented Nov 23, 2023

improve the code style and make the code more clear

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

@@ -23,8 +23,7 @@ enum LeakType {
/// Disposed and garbage collected later than expected.
gcedLate;

static LeakType byName(String name) =>
LeakType.values.firstWhere((e) => e.name == name);
static LeakType byName(String name) => LeakType.values.byName(name);
}
Copy link
Contributor

@polina-c polina-c Nov 23, 2023

Choose a reason for hiding this comment

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

Good suggestion.

}
theContext.addAll(addedContext);
context ??= {};
context!.addAll(addedContext);
}

LeakReport toLeakReport() => LeakReport(
Copy link
Contributor

Choose a reason for hiding this comment

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

The intent of the original code to avoid ! and to use null-safety.
The new code has '!' and thus moves null-safety validation from compiler to developer, making it error prone.
Can you revert changes in this files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure.

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 used the ! syntax because the context must not be null in this case.

How about this change? I will submit it if it's acceptable.

  void setContext(String key, Object value) {
    context ??= {};
    context?[key] = value;
  }

  void mergeContext(Map<String, dynamic>? addedContext) {
    if (addedContext == null) return;
    context ??= {};
    context?.addAll(addedContext);
  }

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good. Thank you.

@polina-c polina-c merged commit 1ab3140 into dart-lang:main Dec 7, 2023
3 checks passed
@polina-c
Copy link
Contributor

polina-c commented Dec 7, 2023

Merged the change. Feel free to create new PR with others.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants