-
Notifications
You must be signed in to change notification settings - Fork 8
refactor!: rework states and adapt spec v0.8 #117
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
Conversation
fabriziodemaria
left a comment
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 good! Let's just make sure to align on the main APIs together with the Swift counterpart before shipping
e80c6dc to
75b793e
Compare
|
Notes:
|
d1bd76d to
4eb640b
Compare
Signed-off-by: Nicklas Lundin <[email protected]> Signed-off-by: Nicklas Lundin <[email protected]> fix: mark as throws Signed-off-by: Nicklas Lundin <[email protected]> Signed-off-by: Nicklas Lundin <[email protected]> Signed-off-by: Nicklas Lundin <[email protected]> Signed-off-by: Nicklas Lundin <[email protected]> fix: add extra buffer capacity for the status flow Signed-off-by: Nicklas Lundin <[email protected]> fix: short circuit on fatal/non ready status refactor!: align setProvider API naming with Swift SDK Signed-off-by: Nicklas Lundin <[email protected]>
Signed-off-by: Nicklas Lundin <[email protected]>
Signed-off-by: Nicklas Lundin <[email protected]>
80173dc to
3c29502
Compare
|
Unfortunately at the current state, the Eventing aspect was removed from the SDK. |
nickybondarenko
left a comment
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.
Really appreciate the documentation! Great work!
fabriziodemaria
left a comment
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.
LGTM 🚀
beeme1mr
left a comment
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.
Will events be re-added before releasing the update?
Signed-off-by: Nicklas Lundin <[email protected]>
3de7f2c to
abb3451
Compare
Yeah, that's my plan. |
Aligning with Spec 0.8.0
This PR
Moves all status handling to the SDK and relies on how the Provider respond to different API calls to expose the correct status.
There are multiple breaking changes in this PR since there are multiple places where the Provider APIs are changed to be suspending functions and the provider is also expected to throw exceptions in the case of failures.
There are big wins for the Provider authors here since there is no need for the provider to emit the statuses using the EventHandler, thus this class is removed.
Aligns with open-feature/swift-sdk#46
Notes on Breaking changes
Provider author breakage
initializeandonContextSetare now suspending and are expecting to throw exceptions on errors.observe()in favour of event handling via SDK calls.SDK consumer breakage
FeatureProvider.awaitReadyOrError()extension function