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

luci-app-upnp: Critical hotfix revert 7a7f9ec and fix old translations #7444

Closed

Conversation

Self-Hosting-Group
Copy link

  • Hotfix that reverts commit 7a7f9ec, which seemed cosmetic and unimportant, but changes defaults and disables both protocols/(service) on next save/apply
  • Manually add the IPv6 always allowed note (in english) to the translations, as this is important, and fix the old texts that were not intended to be carried over but were committed in 9c403f5 and 2e46cfc, and re-remove the obsolete translations restored by the additional commits

For the intro/ACL texts (and some others), the consideration was that the original text was already old, had been changed several times by developers rather than translators, and had undergone four changes since the 23.05 release (missing IPv6 hint, ACLs->ACL, redirected->mapped, shortened sentence), so I decided to have them retranslated to maintain the meaning and integrity of the translations. I think there should be a policy that if e.g. security relevant or the meaning or multiple parts of a sentence have changed, a new translation should be enforced.

IMHO, it was not worth the effort/trouble (the 2nd time) of the additional hasty untested code/commits shortly after merging PR #7407, also because it introduced buggy code that caused a regression and added old text that should have been retranslated, also when I see how fast the translation works. The newly added strings were translated into chinese in less than 6 hours after the PR was merged, now 100% in four languages.

I think we can say that the LuCi plugin is now in good shape after this latest hotfix, except for the 24.10 daemon package which has the issue that the nftable rules are not deleted correctly and the port map listing functions always return a port of 0, see miniupnp/miniupnp#768, miniupnp/miniupnp@d07b0a1 and point 4 in #7407 (comment). A fix can be added as a separate commit to openwrt/packages#24988 if desired.

Self-Hosting-Group added 2 commits December 2, 2024 16:30
Revert "luci-app-upnp: remove dangling lines and split translations"
This reverts commit 7a7f9ec, as it introduced a regression.

Close openwrt#7444

Signed-off-by: Self-Hosting-Group <[email protected]>
Close openwrt#7444

Signed-off-by: Self-Hosting-Group <[email protected]>
@Self-Hosting-Group
Copy link
Author

@systemcrash: Ping. Please merge this functionality hotfix as soon as possible, as I don't want to get user complaints and have to do the manual translation adaptions again, as used the latest strings from the just merged weblate update.

@systemcrash
Copy link
Contributor

Please drop all changes except for Host, Port, and .default inclusion.

@systemcrash
Copy link
Contributor

And that return statement

systemcrash pushed a commit that referenced this pull request Dec 2, 2024
Close #7444

Signed-off-by: Self-Hosting-Group <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants