Conversation
GiteaBot
added
the
lgtm/need 2
github-actions
bot
added
modifies/api
wxiaoguang
left a comment
wxiaoguang
left a comment
There was a problem hiding this comment.
The tests written by AI are not right
GiteaBot
added
lgtm/blocked
I am not done still trying to understand that old bad code lol |
|
Suggest to do it step by step with a plan. Design the table for the future, but the first step should refactor the existing code first. |
bircni
force-pushed
the
feature/restart-notifications
branch
from
ca66385 to
d035e0d
Compare
Co-authored-by: Claude (Opus 4.6) noreply@anthropic.com
Co-authored-by: Claude (Opus 4.6) noreply@anthropic.com
made it like you suggested with the unique |
GiteaBot
added
lgtm/need 2
| repoIDIndex.AddColumn("repo_id") | ||
| indices = append(indices, repoIDIndex) | ||
|
|
||
| statusIndex := schemas.NewIndex("idx_notification_status", schemas.IndexType) |
There was a problem hiding this comment.
Why this index removed. I think it's important.
| @@ -43,6 +46,9 @@ func (opts FindNotificationOptions) ToConds() builder.Cond { | |||
| if opts.IssueID != 0 { | |||
There was a problem hiding this comment.
Since now we are using a unique key, the search condition should use that key instead of this one, otherwise it will become very slow.
There was a problem hiding this comment.
Well, Maybe it's better to keep unique key as an internal condition so that the options could still have the same fields like before but when building the condition, we can use unique key rather than the issue id?
4 participants
Restarts #34803
@lunny :
This PR improved notifications.