Conversation
server/handler/render.go
Outdated
| type errObj struct { | ||
| Title string `json:"title"` | ||
| } | ||
| // TODO: it should be private |
There was a problem hiding this comment.
imo, don't need to be private.
There was a problem hiding this comment.
I made it private because nobody from the outside of the package should write in the response; should it?
There was a problem hiding this comment.
I can easily imagine when we would use it.
For example, we would create an endpoint to download diff or file.
func GetFile(r, w) {
name = getFileName(r)
f, err := getFile(name)
if err != nil {
Write(w, Error{"file with name %s doesn't exist", name})
}
http.ServeContent(w, f)
}There was a problem hiding this comment.
but... that endpoint should be a handler, right?
If so, it can use handler.write too, as done by handler.OauthCallback
76ed472#diff-3ad2ebeb7ccf861129aa87b26fa1f1d8R31
There was a problem hiding this comment.
ahhhh. you moved it from serializer to handler. then all good!
server/serializer/serializers.go
Outdated
| func (e httpError) Error() string { | ||
| if msg := e.Title; msg != "" { | ||
| return msg | ||
| } else if msg = http.StatusText(e.Status); msg != "" { |
There was a problem hiding this comment.
for the future: you don't need to use else if you return before
There was a problem hiding this comment.
It was done to reuse the msg implicit definition made by the first if.
Anyhow, there is no necessity of reusing it, so I'll separate it into two different if, and redefine the msg
smacker
left a comment
There was a problem hiding this comment.
I saw few issues (for example about logger), but we can fix it later. Good to go!!!!!
|
LGTM |
c9a3151 to
9983b3f
Compare
|
I rebased 9983b3f but I couldn't merge because this PR depends on #43 I also added 4f445cd with |
Added after PR review: - Naming conventions as suggested by @smacker - Separate if/return/else as suggested by @smacker - Move assignmentRequest to the handler that uses it - Make the handler.write and serializer.countResponse private - Avoid nil serializer.Response - go-vet. Rename id>ID - go-vet. Avoid unkeyed structs
9983b3f to
f815af0
Compare
supersedes #16
depends on #43
partially solves #3
This PR prepares the skeleton of the backend api serving mocks, using the requirements defined by #3, and following the agreements on the implementations as we talked irl
If approved, the things that will be done in the next PRs will be: