-
-
Notifications
You must be signed in to change notification settings - Fork 12
feat!: update methods to apply to a single provider #287
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 Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #287 +/- ##
==========================================
- Coverage 98.91% 98.90% -0.01%
==========================================
Files 11 11
Lines 460 457 -3
Branches 113 111 -2
==========================================
- Hits 455 452 -3
Misses 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
could you help me understand what you think is too opinionated here? are you thinking of a non serial strategy? |
|
For me, unifont's purpose is to initialize all providers then be able to retrieve fonts data from them through a unified API. The opinionated bit I'd like to get rid of is the fact that you can a given method for all providers at once, that's something I don't do in Astro. I think it's simpler and gives more control to users if they can call eg. Happy to discuss the API, maybe it can be updated but really the point of this PR is to remove the loop in each unifont method |
|
understood. what about an optional second argument which is a provider name? without it, it searches all providers? |
|
This is the current API with providers as the 3rd argument isn't it? To be clear I know what I'm proposing in this PR is not required per say, it's possible to scope to one provider already. I guess the main argument against it is backward compat but since we're in v0 and with very few dependent packages, I thought that could be fine |
unifontwas extracted from@nuxt/fontsand inherited some of its APIs. I personally think it is too opinionated, from my experience using it in Astro.I am aware this a breaking change but unifont is still in v0 so if downstream projects consume this package with
~range, all should be fine.This PR suggests a simpler API that still allows downstream consumers to replicate the previous behavior using the new
unifont.providersAPI:Changes are documented in the README