-
-
Notifications
You must be signed in to change notification settings - Fork 165
Add new option parse_animation
to parse metadata for animated images
#676
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #676 +/- ##
==========================================
+ Coverage 97.55% 97.62% +0.06%
==========================================
Files 21 21
Lines 819 843 +24
==========================================
+ Hits 799 823 +24
Misses 20 20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
8c3f16c
to
c126a82
Compare
packages/core/lib/generators/integrations/image_integration.dart
Outdated
Show resolved
Hide resolved
@@ -116,12 +126,20 @@ ${isPackage ? "\n static const String package = '$packageName';" : ''} | |||
|
|||
@override | |||
String classInstantiate(AssetType asset) { | |||
final info = parseMetadata ? _getMetadata(asset) : null; | |||
final info = parseMetadata || parseAnimation ? _getMetadata(asset) : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the user puts parse_metadata: false
and parse_animation: true
at the same time? We used to imply that parse_metadata
is false, but it seems a bit overwhelming, and we could throw an exception here rather than keep implying these behaviors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parse_animation: true
implies parse_metadata: false
. Parsing animation requires image meta data. We can choose to drop meta data in this case. However, as the meta data has been parsed, I feel it's ok to write the parsed meta data in generated files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we go further and let parse_metadata: true
include generating animations without using this extra option?
packages/core/lib/generators/integrations/image_integration.dart
Outdated
Show resolved
Hide resolved
Also the workflow is complaining about untracked changes |
Co-authored-by: Alex Li <[email protected]>
Co-authored-by: Alex Li <[email protected]>
Co-authored-by: Alex Li <[email protected]>
1382b0f
to
010cae8
Compare
All issues and comments should have been addressed or replied. Please take a look at these changes. Thanks. |
What does this change?
Fixes #675
Type of change
Please delete options that are not relevant.
Checklist:
Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
melos run test
)melos run format
to automatically apply formatting)