Deprecate --packages flag

[liamappelbe]

This flag takes either the .packages or package_config.json file directly. This is a bit inconvenient, especially since .packages is being discontinued, and package_config.json is inside the .dart_tool directory, which users shouldn't have to know the details of.

Instead we should have a --package flag that accepts the path to the root of the package, and uses package_config.findPackageConfig to locate the package config.

[mit-mit]

I think we can go ahead and not only deprecate, but fully discontinue/remove this flag. That is what we're doing elsewhere (and what breaking change dart-lang/sdk#48272 announced).

[liamappelbe]

Ok, I'll delete the --packages flag. But I think I'll still keep support for passing the .packages or package_config.json file into the resolver API, until we do the next major version, since removing support for that is a breaking change.

[mit-mit]

@liamappelbe we're doing a breaking change. It's already been announced and approved. So you can go ahead and remove.

[liamappelbe]

Are you saying this breaking change also covers breaking API changes in package:coverage?

[natebosch]

Dropping support for passing in package_config.json paths entirely (or package paths if we go that route) would be more breaking. The SDK breaking change does not remove without replacement - the replacement already exists.

[liamappelbe]

@mit-mit I think there's 2 questions here:

1. Should we delete the --packages flag, or just deprecate it until the next major version of package:coverage?
2. Should we delete Resolver.packagesPath, or just deprecate it?

I have a PR out that deletes --packages, but just deprecates Resolver.packagesPath, but I'm getting push back on deleting --packages. I don't think the breaking change covers changes to package:coverage, so we should probably just deprecate both of those things, and not delete anything yet.

[mit-mit]

The breaking change is meant to cover all things .packages. We've had the replacement for it (package_config.yaml) for years so as Nate mentioned you can just pass that instead. I don't see why we wouldn't just go ahead and remove any code path that is specific to .packages assuming the corresponding code path based on package_config.yaml is already functional.

[natebosch]

I don't see why we wouldn't just go ahead and remove any code path that is specific to .packages assuming the corresponding code path based on package_config.yaml is already functional.

SGTM.

• Should we delete the --packages flag, or just deprecate it until the next major version of package:coverage?
• Should we delete Resolver.packagesPath, or just deprecate it?

I think both of these are included in the code path for package_config.yaml so removing it shouldn't be necessary. Adding --package-root as another option would be fine, but I don't think we need to delete --packages or existing arguments which will continue to work with the modern file format.

Maybe all we need to do is delete the small chunk of fallback parsing.

https://github.com/dart-lang/coverage/blob/aab92ee30434f131719aae0314cf5413b4997f2d/lib/src/resolver.dart#L98-L111

[liamappelbe]

Ok, so I'll delete the .packages fallback parsing in the resolver, and deprecate (but keep for now) the packagesPath argument in the resolver and the--packages command line flag. Then at the next major version of package:coverage we can delete the deprecated stuff.