Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

add database.NotFoundError helper type#63671

Merged
sqs merged 1 commit into
mainfrom
sqs/db-notfounderror
Jul 5, 2024
Merged

add database.NotFoundError helper type#63671
sqs merged 1 commit into
mainfrom
sqs/db-notfounderror

Conversation

@sqs

@sqs sqs commented Jul 5, 2024

Copy link
Copy Markdown
Member

Use it in the 1 place it seemed obvious. I have other changes where it will be used in more places. This helps standardize our codebase.

Test plan

CI

@sqs sqs requested review from a team and eseliger July 5, 2024 13:42
@cla-bot cla-bot Bot added the cla-signed label Jul 5, 2024
Comment thread internal/database/errors.go Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe cody could write a small test that verifies that this error satisfies the errcode.IsNotFound() check?

Comment thread internal/database/errors.go Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should this type be exported? It's impossible to access noun so no one outside database can create this anyways.
Feels like an implementation detail, no one should ever check for this error, and use the IsNotFound check instead

@sqs sqs force-pushed the sqs/db-notfounderror branch from 676d0fa to 8e1d4db Compare July 5, 2024 13:59
@sqs sqs enabled auto-merge (squash) July 5, 2024 13:59
Use it in the 1 place it seemed obvious. I have other changes where it will be used in more places. This helps standardize our codebase.
@sqs sqs force-pushed the sqs/db-notfounderror branch from 8e1d4db to 69c0ccf Compare July 5, 2024 14:01
@sqs sqs merged commit f6fe8df into main Jul 5, 2024
@sqs sqs deleted the sqs/db-notfounderror branch July 5, 2024 14:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants