Skip to content
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.

Sign up for GitHub

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

Jump to bottom

cleanup locale function usage #27227

Merged
merged 6 commits into from Sep 24, 2023
Merged

cleanup locale function usage #27227

merged 6 commits into from Sep 24, 2023

Conversation

denyskon
Copy link
Member

Throughout the Gitea codebase, you can meet some weird constructions to make locale.Tr work in subtemplates.
Since we now have ctx.Locale.Tr which solves that problem, clean up various templates which pass locale through dict or use some weird constructions like $.root.locale

Going on, it would be great to replace every case of $.locale.Tr and .locale.Tr with ctx.Locale.Tr, but that needs to be done with patience.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 24, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 24, 2023
@github-actions github-actions bot added the topic/ui Change the appearance of the Gitea UI label Sep 24, 2023
@denyskon denyskon added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Sep 24, 2023
@denyskon denyskon added this to the 1.22.0 milestone Sep 24, 2023
@delvh delvh added the performance/speed performance issues with slow downs label Sep 24, 2023
@denyskon
Copy link
Member Author

@delvh I don't know if it really has any impact on speed.....

@delvh
Copy link
Member

delvh commented Sep 24, 2023

I mean…
It directly impacts it as you do the same thing now but don't need to create a new map for every subtemplate call as before, or you need to fill fewer items into the map at least.
I've also thought about applying the memory label, but decided from the two, that it rather affects speed than memory.

It is a fact that this PR will speed up template processing, even if only marginally 😁

delvh
delvh approved these changes Sep 24, 2023
View reviewed changes
templates/repo/issue/view_content/pull.tmpl Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Sep 24, 2023
@delvh
Copy link
Member

delvh commented Sep 24, 2023

Going on, it would be great to replace every case of $.locale.Tr and .locale.Tr with ctx.Locale.Tr, but that needs to be done with patience.

Why with patience?

@delvh
Copy link
Member

delvh commented Sep 24, 2023 •
edited

That is a task that can be automated if ctx.Locale.Tr is really available everywhere.

wxiaoguang
wxiaoguang approved these changes Sep 24, 2023
View reviewed changes
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it doesn't affect performance.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Sep 24, 2023
@wxiaoguang wxiaoguang self-requested a review September 24, 2023 15:39
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Sep 24, 2023
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Sep 24, 2023
wxiaoguang
wxiaoguang requested changes Sep 24, 2023
View reviewed changes
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to narrow the data passed into sub-templates.

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Sep 24, 2023
@delvh delvh mentioned this pull request Sep 24, 2023
Merged
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Sep 24, 2023
@wxiaoguang wxiaoguang removed the performance/speed performance issues with slow downs label Sep 24, 2023
@denyskon denyskon added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 24, 2023
@techknowlogick techknowlogick enabled auto-merge (squash) September 24, 2023 20:24
@techknowlogick techknowlogick merged commit 2325fe7 into go-gitea:main Sep 24, 2023
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 24, 2023
@wxiaoguang
Copy link
Contributor

If it doesn't get backported, some fixes in the future might become non-auto-backportable.

@techknowlogick techknowlogick added the backport/v1.21 This PR should be backported to Gitea 1.21 label Sep 24, 2023
@GiteaBot GiteaBot mentioned this pull request Sep 24, 2023
Merged
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Sep 24, 2023
@denyskon @GiteaBot
cleanup locale function usage (go-gitea#27227)
b66c875
@GiteaBot GiteaBot added the backport/done All backports for this PR have been created label Sep 24, 2023
silverwind pushed a commit that referenced this pull request Sep 25, 2023
@GiteaBot @denyskon
cleanup locale function usage (#27227) (#27240)
daaf0ad 
Backport #27227 by @denyskon

Throughout the Gitea codebase, you can meet some weird constructions to
make `locale.Tr` work in subtemplates.
Since we now have `ctx.Locale.Tr` which solves that problem, clean up
various templates which pass `locale` through `dict` or use some weird
constructions like `$.root.locale`

Going on, it would be great to replace every case of `$.locale.Tr` and
`.locale.Tr` with `ctx.Locale.Tr`, but that needs to be done with
patience.

Co-authored-by: Denys Konovalov <kontakt@denyskon.de>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Sep 25, 2023
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
294a64d 
* giteaofficial/main:
  Add missing public user visibility in user details page (go-gitea#27246)
  Use mask-based fade-out effect for `.new-menu` (go-gitea#27181)
  [skip ci] Updated translations via Crowdin
  Fix z-index on markdown completion (go-gitea#27237)
  Update database-preparation and add note re: MariaDB (go-gitea#27232)
  cleanup locale function usage (go-gitea#27227)
  Fix EOL handling in web editor (go-gitea#27141)
  Fix PushEvent NullPointerException jenkinsci/github-plugin (go-gitea#27203)
  fix issues on action runners page (go-gitea#27226)
  Fix Fomantic UI dropdown icon bug when there is a search input in menu (go-gitea#27225)
  Update go-enry to 2.8.5 (go-gitea#27215)
  Update nodejs installation method in release container (go-gitea#27207)
  Quote table `release` in sql queries (go-gitea#27205)
  Fix push mirror, wrong timestamp format (go-gitea#27153)
  Allow copying issue comment link on archived repos and when not logged in (go-gitea#27193)
  fix: text decorator on issue sidebar menu label (go-gitea#27206)
  Update JS and Poetry dependencies and eslint (go-gitea#27200)
  Remove some dead code (go-gitea#27196)

# Conflicts:
#	templates/repo/issue/view_content/context_menu.tmpl
@denyskon denyskon deleted the refactor/locale branch November 19, 2023 13:50
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Dec 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@delvh delvh delvh approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created backport/v1.21 This PR should be backported to Gitea 1.21 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. topic/ui Change the appearance of the Gitea UI type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Milestone
1.22.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@denyskon @delvh @wxiaoguang @techknowlogick @GiteaBot