-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Penultimate round of db.DefaultContext
refactor
#27414
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other instances I've found:
ComposeMetas
:
user/dashboard/feeds.tmpl
93: {{RenderCommitMessage $.Context .Message $repoLink $.ComposeMetas}}
repo/commit_page.tmpl
22: <h3 class="gt-mb-0 gt-f1"><span class="commit-summary" title="{{.Commit.Summary}}">{{RenderCommitMessage $.Context .Commit.Message $.RepoLink $.Repository.ComposeMetas}}</span>{{template "repo/commit_statuses" dict "Status" .CommitStatus "Statuses" .CommitStatuses "root" $}}</h3>
138: <pre class="commit-body">{{RenderCommitBody $.Context .Commit.Message $.RepoLink $.Repository.ComposeMetas}}</pre>
279: <pre class="commit-body">{{RenderNote $.Context .Note $.RepoLink $.Repository.ComposeMetas}}</pre>
repo/branch/list.tmpl
28: <p class="info gt-df gt-ac gt-my-2">{{svg "octicon-git-commit" 16 "gt-mr-2"}}<a href="{{.RepoLink}}/commit/{{PathEscape .DefaultBranchBranch.DBBranch.CommitID}}">{{ShortSha .DefaultBranchBranch.DBBranch.CommitID}}</a> · <span class="commit-message">{{RenderCommitMessage $.Context .DefaultBranchBranch.DBBranch.CommitMessage .RepoLink .Repository.ComposeMetas}}</span> · {{ctx.Locale.Tr "org.repo_updated"}} {{TimeSince .DefaultBranchBranch.DBBranch.CommitTime.AsTime ctx.Locale}}{{if .DefaultBranchBranch.DBBranch.Pusher}} {{template "shared/user/avatarlink" dict "user" .DefaultBranchBranch.DBBranch.Pusher}}{{template "shared/user/namelink" .DefaultBranchBranch.DBBranch.Pusher}}{{end}}</p>
104: <p class="info gt-df gt-ac gt-my-2">{{svg "octicon-git-commit" 16 "gt-mr-2"}}<a href="{{$.RepoLink}}/commit/{{PathEscape .DBBranch.CommitID}}">{{ShortSha .DBBranch.CommitID}}</a> · <span class="commit-message">{{RenderCommitMessage $.Context .DBBranch.CommitMessage $.RepoLink $.Repository.ComposeMetas}}</span> · {{ctx.Locale.Tr "org.repo_updated"}} {{TimeSince .DBBranch.CommitTime.AsTime ctx.Locale}}{{if .DBBranch.Pusher}} {{template "shared/user/avatarlink" dict "user" .DBBranch.Pusher}} {{template "shared/user/namelink" .DBBranch.Pusher}}{{end}}</p>
repo/commits_list_small.tmpl
41: <span class="gt-mono commit-summary {{if gt .ParentCount 1}} grey text{{end}}" title="{{.Summary}}">{{RenderCommitMessageLinkSubject $.root.Context .Message ($.comment.Issue.PullRequest.BaseRepo.Link|Escape) $commitLink $.comment.Issue.PullRequest.BaseRepo.ComposeMetas}}</span>
46: <pre class="commit-body gt-hidden">{{RenderCommitBody $.root.Context .Message ($.comment.Issue.PullRequest.BaseRepo.Link|Escape) $.comment.Issue.PullRequest.BaseRepo.ComposeMetas}}</pre>
repo/graph/commits.tmpl
32: <span>{{RenderCommitMessage $.Context $commit.Subject $.RepoLink $.Repository.ComposeMetas}}</span>
repo/commits_list.tmpl
63: <span class="commit-summary {{if gt .ParentCount 1}} grey text{{end}}" title="{{.Summary}}">{{RenderCommitMessageLinkSubject $.Context .Message $commitRepoLink $commitLink $.Repository.ComposeMetas}}</span>
71: <pre class="commit-body gt-hidden">{{RenderCommitBody $.Context .Message $commitRepoLink $.Repository.ComposeMetas}}</pre>
repo/diff/compare.tmpl
197: <span id="issue-title">{{RenderIssueTitle $.Context .PullRequest.Issue.Title $.RepoLink $.Repository.ComposeMetas}}</span>
AllowsPulls
:
repo/branch/list.tmpl
128: {{else if and (not .DBBranch.IsDeleted) $.AllowsPulls (gt .CommitsAhead 0)}}
134: {{if and (not .DBBranch.IsDeleted) $.AllowsPulls (gt .CommitsAhead 0)}}
Ping for other reviewers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not feel safe to change the template-related code too much (there are many regressions).
In most (all) cases, the methods used by templates don't need to strictly follow the database context because they can't be in a transaction.
To (partially) resolve the "template code is dynamic" problem, using unique and longer names is the only choice. That's why I keep calling out to use at least 2 words for method/variable names. Then many .Link
regressions could be avoided. And the changed functions should be searched in all templates when refactoring.
And to make the release progress safe, the refactoring time is also important. It's better to do it when the last release is stable enough, and avoid backporting large refactoring. Then people would have enough time to eat the dogfood.
Since the refactoring has started, it shouldn't stop halfway, so I vote my approval here. Hopefully the regressions could be fixed ASAP.
Dashboard does not render any more for me. Is it related?
If this is a valid issue, I really wonder how that could have slipped through. |
It is related. This variable has the same name as a function I changed.
I'm also not sure about it, but the current refactoring don't land in 1.21, so we have enough time to find problems.
I recently had a database deadlock, because one function called in a transaction was using Luckily the next round will likely be the last. |
We need at least some test coverage for typical pages which have been recently regressed with these refactors, like frontpage, pull request view etc. Should be complex scenarios with all the states in them etc. |
* giteaofficial/main: [skip ci] Updated translations via Crowdin Keep filter when showing unfiltered results on explore page (go-gitea#27192) Don't show Link to TOTP if not set up (go-gitea#27585) Fix data-race bug when accessing task.LastRun (go-gitea#27584) Fix template bug (go-gitea#27581) Replace ajax with fetch, improve image diff (go-gitea#27267) Replace assert.Fail with assert.FailNow (go-gitea#27578) Fix the robots.txt path show manual cron run's last time (go-gitea#27544) fully replace drone with actions (go-gitea#27556) Revert "Simplify `contrib/backport` (go-gitea#27520)" (go-gitea#27566) Align ISSUE_TEMPLATE with the new label system (go-gitea#27573) Penultimate round of `db.DefaultContext` refactor (go-gitea#27414)
Part of #27065