Skip to content

feat: STALE state, run handlers for current state immediately, provider name#196

Merged
toddbaert merged 9 commits intomainfrom
fix/clarity-and-events
Aug 15, 2023
Merged

feat: STALE state, run handlers for current state immediately, provider name#196
toddbaert merged 9 commits intomainfrom
fix/clarity-and-events

Conversation

@toddbaert
Copy link
Copy Markdown
Member

@toddbaert toddbaert commented Jun 23, 2023

  • Adds additional clarity regarding provider/client name mapping, as well as intent
  • Replaces client-name for provider-name in events-details
  • adds "STALE" state
  • handlers for ANY state run immediately if provider is in that state

The only functional changes here are small ones related to events, and a new provider state, so particularly interested in @Kavindu-Dodan and @lukas-reining 's opinions on this one.

Relates to #200, which depends on STALE state.

Comment thread specification/sections/05-events.md
Comment thread specification.json
Comment thread specification/sections/01-flag-evaluation.md
Comment thread specification/sections/01-flag-evaluation.md Outdated
Comment thread specification/sections/01-flag-evaluation.md
Comment thread specification.json Outdated
beeme1mr
beeme1mr previously approved these changes Jul 12, 2023
@toddbaert toddbaert marked this pull request as draft July 20, 2023 16:14
Comment thread specification/sections/05-events.md Outdated
Comment thread specification/sections/05-events.md Outdated
@toddbaert toddbaert requested a review from beeme1mr July 20, 2023 16:51
@toddbaert toddbaert dismissed beeme1mr’s stale review July 20, 2023 16:52

Significant changes added

Comment thread specification/sections/05-events.md Outdated
toddbaert and others added 3 commits August 11, 2023 14:25
* Adds additional clarity regarding provider/client name mapping, as well as intent.
* Requires that READY events only run immediately for clients when the provider is already ready.
* Replaces client-name for provider-name in events-details.

Signed-off-by: Todd Baert <[email protected]>
Co-authored-by: Justin Abrahms <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
@toddbaert toddbaert force-pushed the fix/clarity-and-events branch from 7726130 to 3f56549 Compare August 11, 2023 18:26
Signed-off-by: Todd Baert <[email protected]>
@toddbaert toddbaert marked this pull request as ready for review August 11, 2023 18:33
@toddbaert
Copy link
Copy Markdown
Member Author

@Kavindu-Dodan @lukas-reining @beeme1mr @Kavindu-Dodan I've updated this PR. Please re-review.

The main change is the addition of the STALE provider state, and changes to 5.3.3: instead of just READY events running immediately, now the spec dictates that handlers of any type should run immediately if they are added when the provider is already in the associated state.

Copy link
Copy Markdown
Member

@lukas-reining lukas-reining left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

Signed-off-by: Todd Baert <[email protected]>
@toddbaert toddbaert changed the title fix: additional clarity, events fixes feat: STALE state, run handlers for current state immediately, clarity Aug 15, 2023
@toddbaert toddbaert changed the title feat: STALE state, run handlers for current state immediately, clarity feat: STALE state, run handlers for current state immediately, provider name Aug 15, 2023
@toddbaert toddbaert merged commit 8e8c344 into main Aug 15, 2023
@toddbaert toddbaert deleted the fix/clarity-and-events branch August 15, 2023 14:53
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.

5 participants