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.

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

Remove legacy unmaintained packages, refactor to support change default locale #19308

Merged
merged 6 commits into from
Apr 3, 2022

Conversation

wxiaoguang
Copy link
Contributor

@wxiaoguang wxiaoguang commented Apr 2, 2022

Follows

This PR removes two unmaintained vendor packages i18n and paginater

Changes:

  • Rewrite i18n package with a more clear fallback mechanism. Fix an unstable Tr behavior.
  • To keep minimal change, the new i18n still keeps the old name. In future, many code can be merged together with the translation package.
  • In future there can be a refactoring that allow the template call locale.Tr("the.key") directly, without the redundant $.Lang.
  • Add more tests for i18n.
  • Refactor the legacy Paginater to Paginator, test cases are kept unchanged.

Trivial enhancement (no breaking for end users):

  • Use the first locale in LANGS setting option as the default, requested by a user recently (forgot the ID, maybe in discord): i18n.DefaultLocales.SetDefaultLang(setting.Langs[0]), and there is a log to prevent from surprising users.

The UI with i18n still works as before (of course ...), screenshot:

image

@wxiaoguang wxiaoguang changed the title Refactor legacy Refactor legacy unmaintained packages: i18n/paginater Apr 2, 2022
@wxiaoguang wxiaoguang changed the title Refactor legacy unmaintained packages: i18n/paginater Refactor legacy unmaintained packages, support change default locale Apr 2, 2022
@wxiaoguang wxiaoguang added this to the 1.17.0 milestone Apr 2, 2022
@wxiaoguang wxiaoguang added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Apr 2, 2022
@wxiaoguang wxiaoguang changed the title Refactor legacy unmaintained packages, support change default locale Remove legacy unmaintained packages, refactor to support change default locale Apr 2, 2022
@codecov-commenter

This comment was marked as outdated.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 2, 2022
@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 Apr 2, 2022
@silverwind
Copy link
Member

silverwind commented Apr 2, 2022

One future idea regarding i18n in templates: It should be made a helper function, not something that's passed with the data, because it is a pain in gotemplate to pass that data down to sub-templates. I imagine a lot of templates could be simplified if it were a helper.

modules/paginator/paginator.go Show resolved Hide resolved
modules/paginator/paginator.go Show resolved Hide resolved
modules/translation/i18n/i18n.go Outdated Show resolved Hide resolved
Copy link
Contributor

@Gusted Gusted left a comment

Choose a reason for hiding this comment

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

LGTM! Always great to refactor away packages(less binary size 😉)

@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 Apr 3, 2022
@wxiaoguang wxiaoguang merged commit d242511 into go-gitea:main Apr 3, 2022
@wxiaoguang wxiaoguang deleted the refactor-legacy branch April 3, 2022 09:46
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 3, 2022
* giteaofficial/main:
  Remove legacy unmaintained packages, refactor to support change default locale (go-gitea#19308)
  [skip ci] Updated translations via Crowdin
  Prevent intermittent NPE in queue tests (go-gitea#19301)
  Upgrade xorm/builder from v0.3.9 to v0.3.10 (go-gitea#19296)
  An attempt to sync a non-mirror repo must give 400 (Bad Request) (go-gitea#19300)
  Remove legacy `unknwon/com` package (go-gitea#19298)
  Improve package registry docs (go-gitea#19273)
  A pull-mirror repo should be marked as such on creation (go-gitea#19295)
  Refactor legacy `unknwon/com` package, improve golangci lint (go-gitea#19284)
  Skip frontend ROOT_URL check on installation page, remove unnecessary global var (go-gitea#19291)
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
…lt locale (go-gitea#19308)

Remove two unmaintained vendor packages `i18n` and `paginater`. Changes:
* Rewrite `i18n` package with a more clear fallback mechanism. Fix an unstable `Tr` behavior, add more tests.
* Refactor the legacy `Paginater` to `Paginator`, test cases are kept unchanged.

Trivial enhancement (no breaking for end users):
* Use the first locale in LANGS setting option as the default, add a log to prevent from surprising users.
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants