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

When packageParameterEnabled is enabled, include the package name for path assets #383

Merged
merged 2 commits into from
May 5, 2023

Conversation

blaugold
Copy link
Contributor

@blaugold blaugold commented Apr 18, 2023

What does this change?

This change ensures that when packageParameterEnabled is enabled, assets with unknown mime type, for which only the asset key is generated, can be resolved from outside of the containing package.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

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)
  • Appropriate docs were updated (if necessary)

@blaugold blaugold requested a review from wasabeef as a code owner April 18, 2023 09:56
@blaugold
Copy link
Contributor Author

Sorry, forgot to open an issue first. 😅

@codecov
Copy link

codecov bot commented Apr 18, 2023

Codecov Report

Merging #383 (fe5b081) into main (e57307d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #383   +/-   ##
=======================================
  Coverage   98.26%   98.27%           
=======================================
  Files          20       20           
  Lines         693      694    +1     
=======================================
+ Hits          681      682    +1     
  Misses         12       12           
Impacted Files Coverage Δ
packages/core/lib/generators/assets_generator.dart 98.10% <100.00%> (+<0.01%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@wasabeef wasabeef added this to the 5.3.1 milestone Apr 18, 2023
@blaugold
Copy link
Contributor Author

@wasabeef Friendly ping. Is there anything else I need to do to make this PR ready for merging?

@wasabeef
Copy link
Member

@blaugold
Thank you for your good contribution.
I want to check the effect on other files. Please wait 🙏🏽

@blaugold
Copy link
Contributor Author

Got it. Thanks for letting me know and all your work on OSS in general!

Copy link
Member

@wasabeef wasabeef left a comment

Choose a reason for hiding this comment

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

@blaugold Thanks!! and Sorry late merge.

@wasabeef wasabeef merged commit 186df60 into FlutterGen:main May 5, 2023
@wasabeef wasabeef mentioned this pull request May 5, 2023
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.

None yet

2 participants