CLOUDP-249804: Add Not found to search state machine #1612
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
josvazg
force-pushed
the
CLOUDP-249804/atlassearch-error-states
branch
from
d6b21f7
to
2952807
Compare
josvazg
force-pushed
the
atlassearch-translation
branch
from
eb554be
to
0693424
Compare
josvazg
force-pushed
the
CLOUDP-249804/atlassearch-error-states
branch
from
2952807
to
c49cab4
Compare
s-urbaniak
reviewed
May 30, 2024
s-urbaniak
reviewed
May 30, 2024
s-urbaniak
reviewed
May 30, 2024
s-urbaniak
reviewed
May 30, 2024
josvazg
force-pushed
the
atlassearch-translation
branch
from
0693424
to
ac5d63f
Compare
josvazg
force-pushed
the
atlassearch-translation
branch
from
ac5d63f
to
423f264
Compare
Signed-off-by: jose.vazquez <jose.vazquez@mongodb.com>
josvazg
force-pushed
the
CLOUDP-249804/atlassearch-error-states
branch
from
c49cab4
to
4746526
Compare
helderjs
approved these changes
Jul 1, 2024
s-urbaniak
approved these changes
Jul 1, 2024
Comment on lines
+86
to
88
| inAtlas := stateInAtlas != nil | ||
| inSpec := stateInAKO != nil | ||
|
|
There was a problem hiding this comment.
Why? But it's okay, I guess
There was a problem hiding this comment.
This is for readability. The statements inAtlas or inSpec are affirmative, so they read better both alone if inAtlas or when negated if !inAtlas. Where as empty... is a negative statement, which feels like always reversing logic once or twice when you read emptyInAtlas && !emptyInAKO. For example: !emptyInAKO means just present or inSpec
There was a problem hiding this comment.
Then I'd rename those to be inAtlas and inAKO
There was a problem hiding this comment.
I'd say Spec is more accurate and avoids explaining an acronym. We use AKO too much. But I agree we are not consistent, even in this PR.
igor-karpukhin
approved these changes
Jul 1, 2024
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.
Extending the state machine to handle the not found error case as well explicitly:
All Submissions: