Skip to content
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

Extending package information to asset types other than AssetGenImage. #162

Closed

Conversation

abelokon0711
Copy link

As for now the package information is only passed to the AssetGenImage class as a parameter.

This PR extends the package information to other asset types.

@wasabeef wasabeef self-assigned this Nov 29, 2021
@wasabeef wasabeef self-requested a review November 29, 2021 07:08
@wasabeef
Copy link
Member

I'll be checking this PR.

@abelokon0711
Copy link
Author

abelokon0711 commented Jan 11, 2022

@wasabeef Do you have any update or feedback?

This PR should be especially interesting for projects structured as monorepos using melos.

@wasabeef
Copy link
Member

@abelokon0711 I want the change to package_parameter_enabled to be another PR.

@abelokon0711
Copy link
Author

abelokon0711 commented Jan 13, 2022

@wasabeef I reset the parameter name.

After this PR is completed, I will prepare another PR regarding the change to package_parameter_enabled.

String get packageParameterLiteral =>
flutterGen.assets.packageParameterEnabled ? _packageName : '';
String get packageDependencyLiteral =>
flutterGen.assets.packageDependencyEnabled ? _packageName : '';
Copy link
Member

Choose a reason for hiding this comment

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

Please do this too.

Copy link
Author

Choose a reason for hiding this comment

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

Done!

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, I'll check until this weekend.

Copy link
Author

Choose a reason for hiding this comment

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

Do you have any update on this?

@codecov
Copy link

codecov bot commented Apr 11, 2022

Codecov Report

Merging #162 (7732043) into main (6158197) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #162   +/-   ##
=======================================
  Coverage   98.95%   98.95%           
=======================================
  Files          19       19           
  Lines         574      575    +1     
=======================================
+ Hits          568      569    +1     
  Misses          6        6           
Impacted Files Coverage Δ
packages/core/lib/settings/pubspec.g.dart 100.00% <ø> (ø)
packages/core/lib/generators/assets_generator.dart 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6158197...7732043. Read the comment docs.

@wasabeef
Copy link
Member

@abelokon0711
Thank you very much for your PR. But we solved the issue in another way. Please use v4.2.1.

@wasabeef wasabeef closed this May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants