-
Notifications
You must be signed in to change notification settings - Fork 32
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
feat(dynamite)!: allow clients from the http package #1448
Conversation
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.
Looks really good!
packages/dynamite/dynamite_end_to_end_test/test/authentication_test.dart
Outdated
Show resolved
Hide resolved
packages/dynamite/dynamite_runtime/lib/src/dynamite_client.dart
Outdated
Show resolved
Hide resolved
packages/dynamite/dynamite_runtime/lib/src/dynamite_client.dart
Outdated
Show resolved
Hide resolved
packages/dynamite/dynamite_runtime/lib/src/dynamite_client.dart
Outdated
Show resolved
Hide resolved
making this a draft as there is no timeline yet for the next http package release |
f41398f
to
2cb926e
Compare
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.
The dependency check for the http package is missing in dynamite
Signed-off-by: Nikolas Rimikis <[email protected]>
The useragent should be passed with the baseHeaders instead Signed-off-by: Nikolas Rimikis <[email protected]>
2cb926e
to
9d7d86a
Compare
Signed-off-by: Nikolas Rimikis [email protected]
towards: #788
fixes: #1307
Sorry for the big PR but it couldn't be done in smaller ones.
The fixtures now only include the headers explicitly added and not the ones added by the cleint itself.
I haven't checked every edge case but regarding the NC code (without the reexported dynamite_runtime) the only breaking change should be that the
httpClient
parameter now takes aClient
instead of anHttpClient
.