Notifications - Step 1#429
Conversation
| NotificationStatusRead | ||
| ) | ||
|
|
||
| const ( |
There was a problem hiding this comment.
Just put the constants in a single block
There was a problem hiding this comment.
It's necessary for iota and different types, as discussed below.
| UserID int64 `xorm:"INDEX NOT NULL"` | ||
| RepoID int64 `xorm:"INDEX NOT NULL"` | ||
|
|
||
| Status NotificationStatus `xorm:"SMALLINT INDEX NOT NULL"` |
There was a problem hiding this comment.
Really a separate type for simple integers?
There was a problem hiding this comment.
If the type got additional functions it makes sense to me, but not for the current status
There was a problem hiding this comment.
I also prefer enum, and others too, since I was asked to use this in the previous PR.
|
Generally not bad, but you should select more collections from the db instead of firing lots of small queries |
Updated. No more queries inside the loop. |
| if err := sess.Begin(); err != nil { | ||
| return err | ||
| } | ||
| defer sess.Close() |
There was a problem hiding this comment.
put sess.Close() before sess.Begin()
|
Deprecated by #523 |
First step to resolve #145
This deprecates #321
I plan 2 steps: