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

comments from methane #2

Open
methane opened this issue Jul 12, 2023 · 4 comments
Open

comments from methane #2

methane opened this issue Jul 12, 2023 · 4 comments

Comments

@methane
Copy link

methane commented Jul 12, 2023

No description provided.

@methane
Copy link
Author

methane commented Jul 12, 2023

t.Cleanup(func() {
got.Body.Close()
})

setup系のヘルパー関数とかで、関数出る時に後始末できない場合は Cleanup が便利だけれども、defer で十分なところは defer の方がいい気がします。

@methane
Copy link
Author

methane commented Jul 12, 2023

t.Fatalf("failed to read body: %v", err)

t.Fatalf("failed to read file: %v", err)

最近見てなるほどと思ったTweetを共有します。
https://twitter.com/inancgumus/status/1669577946451918849

エラーを単にラップするときは "failed to" なしにコンテキストだけをつけようってことですね。
コンテキスト情報として、例えば2個目のリンク先のコードだと path をつけるとエラーについての情報が増えます。

@methane
Copy link
Author

methane commented Jul 12, 2023

return fmt.Errorf("cannot create same name user: %w", ErrAlreadyEntry)

細かいけど、nameの重複はエラーじゃないよね。

@pollenjp pollenjp mentioned this issue Jul 12, 2023
@pollenjp
Copy link
Owner

pollenjp commented Jul 13, 2023

@methane ありがとうございます!こちらにそれぞれの返信を書かせていただきました。

#1 (review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

2 participants