Conversation
| @@ -0,0 +1,3 @@ | |||
| sample | |||
There was a problem hiding this comment.
This will panic if the slice is empty.
There was a problem hiding this comment.
Log messages already have timestamps, no need to write it additionally.
There was a problem hiding this comment.
The idiomatic way is to accept interfaces (io.Reader in this case), not concrete types.
There was a problem hiding this comment.
We may have a long list of associated nodes (> 10000), and it will exceed the response limit. I suggest truncating it to the first 100, as we don't really need all of them here.
whisk
force-pushed
the
sample/code-reviews
branch
from
baa32e7 to
a399532
Compare
|
|
||
| import "net/http" | ||
|
|
||
| var data = []byte{ |
There was a problem hiding this comment.
Nit: data is too ambiguous. userRequestPayload?
|
|
||
| func sample() []byte { | ||
| var method string | ||
| if method == http.MethodGet { |
There was a problem hiding this comment.
Nit: switch-case may be more readable than multiple if-else here.
whisk
force-pushed
the
sample/code-reviews
branch
from
a399532 to
7c2ca73
Compare
| return nil | ||
| } | ||
|
|
||
| func (u *User) BillingAddress() string { |
There was a problem hiding this comment.
Returning an empty string here may hide errors. What do you think about returning (string, error)?
| @@ -0,0 +1,3 @@ | |||
| sample | |||
| sample | |||
| sample | |||
There was a problem hiding this comment.
You forgot to make this thread-safe.
There was a problem hiding this comment.
This is not the first time I see you are ignoring errors. Why?
There was a problem hiding this comment.
This is literally the slowest way to concatenate strings.
There was a problem hiding this comment.
This function is not thread-safe and may cause a race when called concurrently.
There was a problem hiding this comment.
Ignored error may lead to invalid configuration if the config file is not found.
There was a problem hiding this comment.
This allocates memory on each iteration; using strings.Builder would avoid that.
1 participant
No description provided.