Skip to content

Add option to not use module name in MetadataProvider #2529

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

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

srujzs
Copy link
Contributor

@srujzs srujzs commented Nov 27, 2024

DDC library bundle format does not provide module names as part of the ModuleMetadata.

  • To support this discrepancy, a flag is added to MetadataProvider to either use the module name or the module path to identify the module.
  • createProvider is added so LoadStrategys can create MetadataProviders based on whether the module format uses the module name.
  • Cleans up some getters and methods in LoadStrategy implementations so that getters are grouped together.

DDC library bundle format does not provide module names as
part of the ModuleMetadata.

- To support this discrepancy, a flag is added to MetadataProvider
to either use the module name or the module path to identify the
module.
- createProvider is added so LoadStrategys can create
MetadataProviders based on whether the module format uses the
module name.
- Cleans up some getters and methods in LoadStrategy implementations
so that getters are grouped together.
Copy link
Contributor

@jyameo jyameo left a comment

Choose a reason for hiding this comment

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

LGTM! 🎉

Copy link
Contributor

@nshahan nshahan left a comment

Choose a reason for hiding this comment

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

LGTM with a couple comments.

@srujzs
Copy link
Contributor Author

srujzs commented Nov 27, 2024

Thanks for the reviews!

@srujzs srujzs merged commit 4fc9181 into dart-lang:main Nov 27, 2024
47 checks passed
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.

3 participants