Skip to content

Conversation

@kevmoo
Copy link
Contributor

@kevmoo kevmoo commented May 31, 2024

  • dedupe the same class in two places
  • renamed it to not clash with FakeCodec defined somewhere else

@kevmoo kevmoo requested a review from goderbauer May 31, 2024 02:47
@github-actions github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label May 31, 2024
@kevmoo kevmoo added the autosubmit Merge PR when tree becomes green via auto submit App label May 31, 2024

import 'dart:ui';

class NoopCodec implements Codec {
Copy link
Member

Choose a reason for hiding this comment

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

It looks like NoOpX is a more common casing convention in the Flutter repo: https://github.com/search?q=repo%3Aflutter%2Fflutter%20Noop&type=code

Suggested change
class NoopCodec implements Codec {
class NoOpCodec implements Codec {

I don't feel strongly about this though


import 'dart:ui';

class NoopCodec implements Codec {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: consider adding an instance static member. I don't feel strongly about this either, feel free to skip.

Example:

FlutterMemoryAllocations._();
/// The shared instance of [FlutterMemoryAllocations].
///
/// Only call this when [kFlutterMemoryAllocationsEnabled] is true.
static final FlutterMemoryAllocations instance = FlutterMemoryAllocations._();

Copy link
Member

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

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

LGTM

@auto-submit auto-submit bot merged commit de26ec8 into master May 31, 2024
@auto-submit auto-submit bot deleted the dry_up_fake_codec branch May 31, 2024 20:24
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants