Track assignee for issue #808
Conversation
lunny
commented
Feb 1, 2017
lunny
commented
lunny
added
the
type/feature
| @@ -40,6 +40,8 @@ const ( | |||
| CommentTypeLabel | |||
| // Milestone changed | |||
| CommentTypeMilestone | |||
| // Assignees changed | |||
| CommentTypeAssignees | |||
There was a problem hiding this comment.
Why plural? I would prefer singular for consistency
There was a problem hiding this comment.
Because currently Gitea only supports one assignee, but github support many. I think Gitea could support many in future. So I keep it plural for the next PR.
| MilestoneID int64 | ||
| OldMilestone *Milestone `xorm:"-"` | ||
| Milestone *Milestone `xorm:"-"` | ||
| OldAssigneeID int64 |
There was a problem hiding this comment.
It would be nice if we didn't have to have both OldMilestoneID/MilestoneID and OldAssigneeID/AssigneeID, since at most one of them will be relevant for any particular issue.
Would it make sense to simply have OldElementID and ElementID fields? If Type==CommentTypeAssignees then OldElementID and ElementID are assignee IDs, if Type==CommentTypeMilestone they are milestone IDs, etc. I see pros and cons, interested to hear your thoughts 😄
There was a problem hiding this comment.
I don't think that's a good idea. The names should have meaning for easy maintaining. The memory usage is not very different.
models/issue_comment.go
Outdated
| // LoadAssignees if comment.Type is CommentTypeAssignees, then load assignees | ||
| func (c *Comment) LoadAssignees() error { | ||
| if c.OldAssigneeID > 0 { | ||
| var oldAssignee User |
There was a problem hiding this comment.
Why not just use GetUserByID(c.OldAssigneeID)?
models/issue_comment.go
Outdated
| } | ||
|
|
||
| if c.AssigneeID > 0 { | ||
| var assignee User |
|
LGTM |
tboerger
added
the
lgtm/need 2
|
Let L-G-T-M work |
tboerger
added
lgtm/need 1
|
@ethantkoenig please confirm. |
|
LGTM |
tboerger
added
lgtm/done
|
resolved #789 |
