Skip to content

migrate platform channels benchmarks #84066

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
Jun 7, 2021
Merged

migrate platform channels benchmarks #84066

fluttergithubbot merged 2 commits into from
Jun 7, 2021

Conversation

@Abhishek01039
Copy link
Contributor

@Abhishek01039 Abhishek01039 commented Jun 6, 2021

part of #84014

@flutter-dashboard flutter-dashboard bot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Jun 6, 2021
@flutter-dashboard
Copy link

flutter-dashboard bot commented Jun 6, 2021

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@google-cla google-cla bot added the cla: yes label Jun 6, 2021
goderbauer
goderbauer reviewed Jun 7, 2021
View reviewed changes
dev/benchmarks/platform_channels_benchmarks/lib/main.dart Outdated
Comment on lines 129 to 130
final ByteData? largeBufferBytes =
const StandardMessageCodec().encodeMessage(largeBuffer);
Copy link
Member

@goderbauer goderbauer Jun 7, 2021

Choose a reason for hiding this comment

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

Suggested change
final ByteData? largeBufferBytes =
const StandardMessageCodec().encodeMessage(largeBuffer);
final ByteData largeBufferBytes =
const StandardMessageCodec().encodeMessage(largeBuffer)!;

dev/benchmarks/platform_channels_benchmarks/lib/main.dart Outdated
);
resetChannel.send(true);
await _runBasicBinary(basicBinary, largeBufferBytes, 1); // Warmup.
await _runBasicBinary(basicBinary, largeBufferBytes!, 1); // Warmup.
Copy link
Member

@goderbauer goderbauer Jun 7, 2021

Choose a reason for hiding this comment

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

remove the ! with the change above.

@Abhishek01039 Abhishek01039 requested a review from goderbauer June 7, 2021 17:28
goderbauer
goderbauer approved these changes Jun 7, 2021
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Jun 7, 2021

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Mac module_test_ios has failed. Please fix the issues identified (or deflake) before re-applying this label.

@goderbauer goderbauer mentioned this pull request Jun 7, 2021
Closed
27 tasks
@fluttergithubbot fluttergithubbot merged commit e6aa6ce into flutter:master Jun 7, 2021
@Piinks Piinks added a: null-safety Support for Dart's null safety feature c: tech-debt Technical debt, code quality, testing, etc. labels Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@goderbauer goderbauer goderbauer approved these changes

Assignees

No one assigned

Labels

a: null-safety Support for Dart's null safety feature c: contributor-productivity Team-specific productivity, code health, technical debt. c: tech-debt Technical debt, code quality, testing, etc.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Abhishek01039 @fluttergithubbot @goderbauer @Piinks