ADD PDResultSuccessEmpty #86
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ninovanhooff
commented
Oct 29, 2024
Nycto
reviewed
Oct 29, 2024
|
So a disadvantage is that you are forced to handle the new result kind also for the two responses that have a non-optional response: getScoreboards and getScores. We could work around this by having two sets of types: PDRequiredResult + PDRequiredResultKind vs PDOptionalResult + PDOptionalResultKind. That feels over-engineered. Feeling lukewarm about that, let's not do it. Unless you know of some fancy Nim feature that lets us express this nicely? @Nycto |
|
Well — let’s start with: what are we modeling here? Is it actually a failed request (like WiFi unavailable)? If so, why don’t the other requests include this response state? |
Yeah, I guess the unavailable response is always relevant. Made the change to Unavailable |
Nycto
reviewed
Nov 7, 2024
Nycto
approved these changes
Nov 8, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By using the Scoreboards API for Wheelsprung, it feels wrong to classify a result without data as an error.
Therefore I added an extra PDResultKind for this and documented the cases where this may be returned to the callback