In D29299#676448, @dfaure wrote:

When you wrote "ki18n_install() is basically used by KF sources that use ECM already" it seemed to me that this was looking at KDE community code only

In D29299#676446, @dfaure wrote:

In D29299#676445, @pino wrote:

I asked for actual valid use cases when using the new variables first would break, and I still got none. There is a limit to how much you can keep broken code working... assuming such broken code exists. I don't think there is any of this such situation, as ki18n_install() is basically used by KF sources that use ECM already, with marble being the only exception (and even that, marble won't break).

As you know, there are KF5-based applications outside the realm of what we can see in LXR.
One of the primary goals of KF5 is to be useable by other applications not written by the KDE community (I actually know quite a few).
As such, it's not hard to imagine a cmake-based application that uses Qt and GNUInstallDirs [with qmake going away this will happen more and more], and one day it wants to use one of the frameworks. At that point, it shouldn't be forced to switch to ECMInstallDirs. Therefore I definitely see value in keeping the two things separate, as long as we keep making things easy for what is the most common case for us: using both.

In D29299#676439, @dfaure wrote:

@pino Other than the fact that you think D29136 is "good enough", do you have any concrete objection to this version?

In T13519#238118, @huftis wrote:

The desktop files would then be .desktop.in files, and intltool (intltool-merge) could be used to generate a .desktop file from the .desktop.in file and the set of .po files – just the way it’s done for normal (non-KDE) applications which store the PO files in the source repository.

In T11070#238091, @subins2000 wrote:

1. Yes, the GNOME system has problems, it simplifies the translator entry, but is difficult for reviewers (who has other jobs) and might even discourages new contributors. We can do better

In T13311#237876, @aspotashev wrote:

Shall we discuss how the system is going to handle multiple translation branches (e.g. trunk/l10n-kf5, branches/stable/l10n-kf5, ...) ?

Here are the obvious options:

1. Use a web-based translation system that natively supports multiple "branches" for .po files. Does anyone know which of the listed systems have such support?
2. Enable PO Summit for all team, then wire the web system up to the .po files gathered (unified/merged) from all branches.
3. ...anything else?

In T13514#237772, @dkazakov wrote:

Well, we can discuss the layout. I don't think that "one repo - one application" will make much download amount for the application translators. We can also split it in "one repo - one language for one application" manner. It might be too granular, but it might work and save time for people who work on all applications at the same time.

In T13514#237769, @dkazakov wrote:

I don't really understand why cannot we use submodules for that? It looks like a submodule can track external branches. Why cannot we just add a submodule into each KDE project that fetches translations from an external repo by tracking some specific branch?

In T13514#237685, @subins2000 wrote:

@pino The goal of doing the git migration is to solve "SVN is harder to use for new contributors". I meant this problem will be better solved by an online tool than a migration to git.

@subins2000 this ticket is NOT about a web-based translation tool, there is a separate ticket for that. Please keep the topic of this focused ONLY on the svn -> git migration.

In T13514#237640, @huftis wrote:

But a giant Git repository with all the languages will be. So please consider using submodules, one for each language.

In T13514#237637, @aspotashev wrote:

In T13514#237629, @pino wrote:

Git should facilitate integration with external translation file trees maintained by Linux distributions (e.g. BaseALT)

@aspotashev can you please explain why these distros care about the layout of our translations? When packaging upstream releases they ought to use translations as available in release tarballs, not pick random files from upstream translation trees.

Our [upstream] translation team has limited capacity and we are unable to review new translations submitted by the distro on a timely manner. Therefore they want to use their own versions of translations when they are not upstreamed yet. They do it by injecting their custom translation files by the distro-specific packaging scripts.

Git should make it easier for them to merge upstreamed translations back into their custom tree when both repos will be in Git.

Git should facilitate integration with external translation file trees maintained by Linux distributions (e.g. BaseALT)

Please keep the eventual integration of Weblate (or any other web-based translation system) out of this "migrate to git" roadmap. Adding a web-based translation system requires additional complexity and even workflow changes that would make doing this conversion a even bigger and risky task than what it already is.

Patch won't apply directly, but you've done the work finding the bug, I can sort that.

In D27633#619369, @aacid wrote:

In D27633#619365, @aacid wrote:

I think this broke https://build.kde.org/job/Applications/job/ktp-common-internals/job/kf5-qt5%20SUSEQt5.12/20/console guess ¿KAccountsDPlugin now requires parameters to the constructor and is thus not a valid Q_INTERFACE?

@nicolasfella can you please look at it?

On top of that that's a BIC change, you can't do BIC changes on KF5 repos, so revert?

In D29805#674218, @meven wrote:

In D29805#674206, @pino wrote:

In D29805#674205, @meven wrote:

In D29805#674204, @pino wrote:

FIXED-IN: 20.08

still for 20.08...

Yes kio-extra is released with KDE Applications

Yes, I know. Asked to land this fix instead to release/20.04, and thus change the commit message accordingly.

Please be explicit when you comment, no one could deduce what you meant.

In D29805#674205, @meven wrote:

In D29805#674204, @pino wrote:

FIXED-IN: 20.08

still for 20.08...

Yes kio-extra is released with KDE Applications

still for 20.08...

please target release/20.04 for this fix, thanks

In D28673#672845, @broulik wrote:

so, can the regexp be replaced or does it need to stay?

Please update the text of the revision and the commit message, as they don't make much sense. Please mention that it fixes a crash in case ddjvu is not installed, by avoiding executing it, and using _exit() to not run atexit handlers in case execvp() fails.

This change is definitely not correct.

Please read what @zzag and me wrote: you must fix the buddy of the label widget, not its name.

@sdepiets bumping dependencies in a stable branch is against the freeze policy:
https://community.kde.org/Schedules/release_service/20.04_Release_Schedule#Thursday.2C_March_12.2C_2020:_20.04_Dependency_Freeze

why not KEncodingProber from the KCodecs framework, like also the commented bits hint about?

The accelerator is correct to use, as it will activate the buddy associated to to label... which is wrong, as quickTileLabel does not exist.
Did you try changing the buddy of quickTileLabel to kcfg_ElectricBorderTiling?

In D29299#660614, @kossebau wrote:

In D29299#660519, @pino wrote:

In D29299#660505, @kossebau wrote:

Because people do strange things, and I prefer to rather not break their card house unless necessary.

Again, in which cases? The only way it might change the path is: use ki18n AND build with cmake AND use the ki18n_install macro AND define a KDE_INSTALL_LOCALEDIR variable yourself. Too many factors to make it even something to even bother thinking about TBH.

Mileages difer :)

One does not need to define KDE_INSTALL_LOCALEDIR oneself. One only needs to use find_package(KF5Plasma) or find_package(KF5Package), because some subrepo also does a plasma applet for Plasma integration or have someone cluelessly copied cmake code together because ENOTIMEFORBUILDSYSTEM and copying from the best=KDE and now-it-builds-so-do-not-touch-again.

In D29299#660505, @kossebau wrote:

Because people do strange things, and I prefer to rather not break their card house unless necessary.

In D29299#660490, @kossebau wrote:

D29136 in the current version though changes behaviour by favouring KDE_INSTALL_LOCALEDIR over LOCALE_INSTALL_DIR. Which at least in theory might somewhere in some distant galaxy break things ;)

In D29299#660466, @kossebau wrote:

In D29299#660465, @pino wrote:

Also, your patch basically includes D29136 in the case of no DESTINATION parameter specified, hence my suggestion is:

• edit D29136 to do the fallback using the same logic introduced here: this way marble is already fixed with no other changes, and ki18n_install will work also with KDE_INSTALL_DIRS_NO_DEPRECATED (e.g. for release-service packages)
• have this to add the DESTINATION parameter, so packages can opt-in to use it if they can/want

Not exactly sure what you mean? Do you want two separate commits/reviews, one per issue?

The patch should not require existing users to adapt

The problem is that ki18n_install() is rarely used manually

The problem is that ki18n_install() is rarely used manually, and generally it is appended by the release scripts to the top-level CMakeLists.txt file that goes into the tarballs.
This means that either the majority of the packages will not make use of this, or a mass-bump of the ki18n dependency (which I'd rather not do).

@tcanabrava @carneirogustavo this change does not look correct to me: now both the setters are effectively no-op if the new values are different than the old ones.
Are you sure it was not meant to be if (m_foo == foo) return; instead?

My concern is that this change is about adding an AppStream file, with no mention of other changes to the desktop file. So it is not clear (especially when looking at this after some time) whether the change was somehow related (which doesn't look like), or maybe it was a local change for testing that slipped in while committing.

Is the change to GenericName intentional?

In D29077#656383, @patrickelectric wrote:

@pino if you can, please the history if of this patch to see my first and working version.
https://phabricator.kde.org/D29077?id=80827

CMake Error at src/app/CMakeLists.txt:87 (install):

(Commented in the wrong place)

Shouldn't the test completely ignore the system location, to avoid interferences from the system installation?

LGTM for me from an i18n POV, I'll let @dvratil give a final review/approval.