final

final

You could make this final, if you remove line 134 and then use packageManifestDescriptor['flutter'] directly in line 138.

=> hashValues(base, assetEntry, relativePath, source);

final

Add trailing ,

Add trailing ,

I'm fine with adding trailing , but this file does not seem to adopt that convention...

Our conventions change over time and we slowly transition the entire codebase over time. Always aim for the most up to date conventions (which ideally are described in the style guide).

You should feel empowered to apply new conventions to old code aggressively. For example I will often pepper trailing commas around the codebase as I'm doing other things.

You should feel empowered to proposed PRs for the Flutter style guide, especially for updating it to match explicit conventions you hear about in reviews, or implicit conventions you observe in the codebase. The style guide isn't complete.

Add trailing ,

Looking at other Flutter implementations of == it seems the convention is to do:

@override
bool operator ==(dynamic other) {
  if (identical(other, this))
    return true;
  if (other.runtimeType != runtimeType)
    return false;
  final _Asset typedOther = other;
  return typedOther.base == base
      && typedOther.assetEntry == assetEntry
      && typedOther.relativePath == relativePath
      && typedOther.source == source;
}

Nit: I'd remove the 'only'. The package argument can also be used in the app implementation, to refer to an asset in an imported package, but it must be specified in the package implementation.

Yes, true.

final flutterManifest = ... ?

IIRC, we usually do something fancier to combine the hashcodes (multiply, xor, or shift), and make sure we stay in the 30-bit range.

I think if you did if (other is! _Asset) return false;, then the analyzer should be smart enough to know that other is an _Asset below, so you wouldn't need the extra variable and cast.

Done

nit: space before ?

trailing comma

update the docs

this should include more details about:
(a) what this does
(b) how you write pubspec.yaml files to use this

(c) how to know what string to use for "package"

same nits for this class as the other class

nit: trailing comma here and elsewhere

include the same information here as earlier regarding what package does, how to use it, etc.

ditto

nit: indent by two less

nit: compare runtimeType

nit: vertically align the expressions (you can put the && at the start of the line too to make it prettier)

nit: never break a line that contains =>

use hashValues, not xor.

I cannot use hashValues from dart:ui since the library is not available on the stand-alone VM. Do we have something else I can use instead?

Hm, good point. I guess xor is probably fine. dart-lang/sdk#25217 is the bug for getting hashValues into dart:core... :-)

style: if there's a line break after the (, there should be one before the )

Use of => with multi-line body should be avoided, e.g. by replacing with

@override
int get hashCode => base.hashCode ^ assetEntry.hashCode ^ relativePath.hashCode ^ source.hashCode;

or

@override
int get hashCode {
  return base.hashCode
       ^ assetEntry.hashCode 
       ^ relativePath.hashCode 
       ^ source.hashCode;
}

Indent by 2 less.

You can improve vertical alignment and consistency by always mentioning other first.

Cut extra space before ##...

... and paste it here.

Remove extra line?

Above -- where?

assets

'packages/<package_name>/'

nitty nit nit: we really prefer it when the code is vertically aligned cleanly, as in:

    return base.hashCode
         ^ assetEntry.hashCode
         ^ relativePath.hashCode
         ^ source.hashCode;
```

Remove extra //.

Maybe move this to an establishFlutterRoot function so you don't have to duplicate the comment in every test.

Maybe extract this loop as a writeAssets function too?

Move first and second parameter to their own lines.