Skip to content
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

Minor: use Option rather than Result for not found suggestion #12512

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Sep 17, 2024

Which issue does this PR close?

Related to #12464

follow on ot #12469

Rationale for this change

Implement @comphead 's suggestion #12469 (comment)

Wondering may be it can be an Option?
Usually find methods, on collections as well returns the Option

What changes are included in this PR?

Are these changes tested?

yes, covered by existing unit tests

Are there any user-facing changes?

Technically it is an API change, but the API has not yet been released

@github-actions github-actions bot added the sql SQL Planner label Sep 17, 2024
@alamb alamb changed the title Alamb/use option Minor: use Option rather than Result for not found suggestion Sep 17, 2024
Copy link
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @alamb

Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

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

lgtm 💪

@comphead comphead merged commit aeca7ea into apache:main Sep 17, 2024
24 checks passed
@alamb alamb deleted the alamb/use_option branch September 18, 2024 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sql SQL Planner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants