@prologic and others may I ask you to review some code? https://git.envs.net/cblte/golang-webapp/releases/tag/0.7 It is not finished yet, but I think I am on a good way. Still needs some polishing. WebDev is actually new to me with all the status codes and stateless things and everything. So feedback would be appreciated. Using Redis because it was easy to setup and to implement. Actually want to use SQLite or something else, but Redis seems to do the job for now.
Hello @carsten@yarn.zn80.net, I’ll say that :
- everything being in the main is bad
- the handler structure can be better, more structured and separating the http part, from the real business part (some interfaces may help)
- rclient should be injected.
- there is no env variable management / config management (viper is good for that)
- there is no unit tests (that one you know)
- there is no metrics (if it was a professional project I ’ll be waiting for those)
- gitignore should maybe have golang-webapp on it.
@tkanos’s comments are all good points. I actually think the code is actually pretty good really. One thing you can also do to enhance deployment(s) is the use of the embed package to embed the static assets and templates. Freel free to borrow/slash/steal code from yarnd
😅
@prologic Thanks. This embed package is something I was looking for. Have it on my list, but did not get that far
@carsten@yarn.zn80.net I only had a quick look and this is all in no particular order:
- I’m sure
go fmt
would add a space after the comment marker//
. - Go doc strings are supposed to start with the name of the variable etc (I’m not a fan of this, either).
- Sometimes
log.SetPrefix(…)
ends with a space, sometimes not. - Some messages start capital, some don’t.
- Typo:
occurred
with doubler
. - On lots of errors no appropriate status code is set.
- Some
err
can be scoped in theif
like that:if err := foo(); err != nil { … }
- The
<title>
s could be improved. - I have no idea about redis, but
rclient.Set("user:"+username, …)
looks suspicious to me and reminds me of SQL injections. 10.go cookie, err := r.Cookie("session_token") if err != nil { if err == http.ErrNoCookie { … return } }
doesn’t look complete. Also handle other errors? Or simplify without nil check.