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

refactor: using error is instead of == [skip changelog] #10093

Merged
merged 6 commits into from
Aug 22, 2023
Merged

refactor: using error is instead of == [skip changelog] #10093

merged 6 commits into from
Aug 22, 2023

Conversation

kehiy
Copy link
Contributor

@kehiy kehiy commented Aug 21, 2023

I used errors.Is() instead of == or != and also use the %w for wrapped errors.

@kehiy kehiy requested a review from a team as a code owner August 21, 2023 21:20
Copy link
Contributor

@Jorropo Jorropo left a comment

Choose a reason for hiding this comment

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

I wont comment on all the occurences, but io.EOF must never be wrapped because it was created before errors.Is and there is way to much code that relies on ==.
errors.Is also generates garbage so we shouldn't use it unless it is realistical to trigger where == would not (so not io.EOF).

client/rpc/apifile.go Outdated Show resolved Hide resolved
@kehiy
Copy link
Contributor Author

kehiy commented Aug 22, 2023

@Jorropo so, I need to remove all errors.Is() ? or just for io.EOFs?

@Jorropo
Copy link
Contributor

Jorropo commented Aug 22, 2023

io.EOF and probably io.ErrUnexpectedEOF too

@kehiy
Copy link
Contributor Author

kehiy commented Aug 22, 2023

@Jorropo its fixed

@kehiy kehiy requested a review from Jorropo August 22, 2023 12:02
@kehiy
Copy link
Contributor Author

kehiy commented Aug 22, 2023

@Jorropo can you have a review?

Copy link
Contributor

@Jorropo Jorropo left a comment

Choose a reason for hiding this comment

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

Last one please and lgtm

core/bootstrap/bootstrap.go Outdated Show resolved Hide resolved
Co-authored-by: Jorropo <[email protected]>
@kehiy
Copy link
Contributor Author

kehiy commented Aug 22, 2023

@Jorropo donee

Copy link
Contributor

@Jorropo Jorropo left a comment

Choose a reason for hiding this comment

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

LGTM thx

@Jorropo Jorropo enabled auto-merge (rebase) August 22, 2023 14:47
@Jorropo Jorropo disabled auto-merge August 22, 2023 14:50
@Jorropo Jorropo enabled auto-merge (squash) August 22, 2023 14:50
@Jorropo Jorropo merged commit 2b7c20f into ipfs:master Aug 22, 2023
16 checks passed
@kehiy kehiy deleted the branch branch August 22, 2023 14:54
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.

2 participants