-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bug: fix assignees double load bug #10856
Conversation
Because the assigness has been loaded in compare.go 416: RetrieveRepoMetas(ctx, ctx.Repo.Repository, true) then issue.go 381 RetrieveRepoMilestonesAndAssignees(ctx, repo) then issue.go 361 -- 365 , they are load assignees So the code on compare.go 425 -- 427 is double work, and which is the reason of go-gitea#10853 Signed-off-by: a1012112796 <[email protected]>
Ok so the function you're looking at is: Lines 374 to 442 in 90919bb
In particular you suggest removing lines: Lines 425 to 428 in 90919bb
As they are duplicated by the call here: Line 416 in 90919bb
I.e. Lines 368 to 397 in 9d3e69e
Which calls: Lines 347 to 366 in 9d3e69e
In particular Line 361 in 9d3e69e
|
Now the trouble is Line 393 in 90919bb
And Line 414 in 90919bb
So it's not always a necessarily duplicate call. (Although it may be a completely unnecessary call ... I'm looking at this code likely for the first time from my phone and bed so I don't know completely.) Would it be sufficient to add the opposite conditions on the ctx.Data['Assignees'] bit? Or is this call totally unnecessary in the first place? |
Hello, I think this two check is all to check if it's ready to make a new pr, if not , It's not necessary to load them. because It's not useed in compare page. thanks. you can see them in compare.tml :
and gitea/templates/repo/diff/compare.tmpl Line 60 in 7cd4704
|
OK the Assignees box is only shown if those two conditions are passed: gitea/templates/repo/diff/compare.tmpl Lines 60 to 83 in 7cd4704
in particular the Assignees box comes from: gitea/templates/repo/diff/compare.tmpl Line 78 in 7cd4704
Therefore the get assignees call is definitely unnecessary for every other case and LGTM. |
Codecov Report
@@ Coverage Diff @@
## master #10856 +/- ##
=======================================
Coverage 43.41% 43.41%
=======================================
Files 593 593
Lines 83265 83262 -3
=======================================
+ Hits 36150 36152 +2
+ Misses 42622 42619 -3
+ Partials 4493 4491 -2
Continue to review full report at Codecov.
|
Because the assigness has been loaded in
compare.go 416:
RetrieveRepoMetas(ctx, ctx.Repo.Repository, true)
then
issue.go 381
RetrieveRepoMilestonesAndAssignees(ctx, repo)
then
issue.go 361 -- 365 , they are load assignees
So the code on compare.go 425 -- 427 is double work,
and which is the reason of #10853
close #10853