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

Refactored the AssetGenImage code to be another Integration #432

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

bramp
Copy link
Contributor

@bramp bramp commented Oct 14, 2023

What does this change?

Refactored the AssetGenImage code to be another Integration, so that it looks more similar to SVG/Lottie/Rive/etc.

This simplifies the assets_generator, and no longer forces AssetGenImage to be output even when there are no images.

This can help ensure all integration are on level footing (perhaps helping with task 1 in this list #384)

Type of change

  • Refactor to cleanup the code base.

Checklist:

Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open a GitHub issue as a bug/feature request before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
    • Ensure the tests (melos run unit:test)
    • Ensure the analyzer and formatter pass (melos run format to automatically apply formatting)

Copy link

codecov bot commented Nov 4, 2023

Codecov Report

Merging #432 (12892d7) into main (ba28410) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #432   +/-   ##
=======================================
  Coverage   98.28%   98.29%           
=======================================
  Files          20       21    +1     
  Lines         700      702    +2     
=======================================
+ Hits          688      690    +2     
  Misses         12       12           
Files Coverage Δ
packages/core/lib/generators/assets_generator.dart 98.04% <100.00%> (-0.11%) ⬇️
...lib/generators/integrations/flare_integration.dart 100.00% <100.00%> (ø)
...lib/generators/integrations/image_integration.dart 100.00% <100.00%> (ø)
.../core/lib/generators/integrations/integration.dart 100.00% <100.00%> (ø)
...ib/generators/integrations/lottie_integration.dart 92.85% <100.00%> (-0.25%) ⬇️
.../lib/generators/integrations/rive_integration.dart 100.00% <100.00%> (ø)
...e/lib/generators/integrations/svg_integration.dart 100.00% <100.00%> (ø)
packages/core/lib/settings/asset_type.dart 95.83% <ø> (-1.05%) ⬇️

Copy link
Member

@AlexV525 AlexV525 left a comment

Choose a reason for hiding this comment

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

Looks cool!

@bramp bramp force-pushed the image_integration branch 2 times, most recently from d87f99d to 2f1d80e Compare November 13, 2023 18:57
…ooks more similar to svg/lottie/etc.

This simplifies the assets_generator, and no longer forces AssetGenImage to be output even when there are no images.
@bramp
Copy link
Contributor Author

bramp commented Nov 13, 2023

Fixed the merge conflict, and improved the docs. Please take a look.

@AlexV525 AlexV525 merged commit 72fa49e into FlutterGen:main Nov 14, 2023
5 checks passed
@wasabeef wasabeef added this to the 5.4.0 milestone Jan 5, 2024
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.

3 participants