Skip to content

Conversation

@craigpastro
Copy link
Member

This PR

Add a stale state and ensure event handlers run immediately if the associated provider and the state associated with the handle are compatible.

Related Issues

Resolves #220.

Notes

Follow-up Tasks

How to test

Signed-off-by: Craig Pastro <pastro.craig@gmail.com>
@craigpastro craigpastro requested a review from a team as a code owner September 12, 2023 21:50
@craigpastro craigpastro mentioned this pull request Sep 12, 2023
6 tasks
@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #221 (617ef2b) into main (d850ebc) will increase coverage by 0.12%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #221      +/-   ##
==========================================
+ Coverage   78.63%   78.76%   +0.12%     
==========================================
  Files          10       10              
  Lines        1203     1210       +7     
==========================================
+ Hits          946      953       +7     
  Misses        230      230              
  Partials       27       27              
Files Changed Coverage Δ
pkg/openfeature/provider.go 65.38% <ø> (ø)
pkg/openfeature/event_executor.go 98.75% <100.00%> (+0.03%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@toddbaert toddbaert changed the title fix: ensure event handlers run immediately if able feat: ensure event handlers run immediately, add STALE (0.7.0 compliance) Sep 13, 2023
Copy link
Contributor

@Kavindu-Dodan Kavindu-Dodan left a comment

Choose a reason for hiding this comment

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

Nice work. LGTM

@toddbaert toddbaert self-requested a review September 13, 2023 18:01
@toddbaert
Copy link
Member

I'll give this a review later today!

@toddbaert toddbaert self-requested a review September 13, 2023 19:27
Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

LGTM with the exception of some apparent copy/paste mistakes in test names/logs.

Signed-off-by: Craig Pastro <pastro.craig@gmail.com>
@toddbaert toddbaert changed the title feat: ensure event handlers run immediately, add STALE (0.7.0 compliance) feat: run event handlers immediately, add STALE (0.7.0 compliance) Sep 14, 2023
@toddbaert toddbaert merged commit 9c0012f into open-feature:main Sep 14, 2023
@craigpastro craigpastro deleted the handlers-run-immediately branch September 14, 2023 13:48
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.

event handlers should run immediately if the associated provider is in the state associated with that handler already

3 participants