This revision was automatically updated to reflect the committed changes.
Closed by commit R240:e6982aba: Introduce fetch-translations build command
(authored by apol).
REPOSITORY
R240 Extra CMake Modules
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D5143?vs=13117=13161
sitter accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R240 Extra CMake Modules
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D5143
To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid, ilic, sitter
Cc: sitter
ltoscano added inline comments.
INLINE COMMENTS
> KDECMakeSettings.cmake:289
> +option(KDE_L10N_AUTO_TRANSLATIONS "Automatically 'make
> fetch-translations`" OFF)
> +set(KDE_L10N_BRANCH "trunk" CACHE STRING "Branch from l10n.kde.org to
> fetch from: trunk | stable | lts | trunk_kde4 |
apol added a comment.
Fixed.
Can I please have a ship it? we can maybe polish it over time
REPOSITORY
R240 Extra CMake Modules
REVISION DETAIL
https://phabricator.kde.org/D5143
To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid, ilic
Cc: sitter
apol marked 3 inline comments as done.
REPOSITORY
R240 Extra CMake Modules
REVISION DETAIL
https://phabricator.kde.org/D5143
To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid, ilic
Cc: sitter
apol updated this revision to Diff 13117.
apol added a comment.
Address sitter's comments
REPOSITORY
R240 Extra CMake Modules
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D5143?vs=13006=13117
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D5143
AFFECTED FILES
sitter added a comment.
needs a version bump in the docs now
beyond that LGTM
INLINE COMMENTS
> KDECMakeSettings.cmake:86
> +#
> +# Since 5.33.0
>
5.34 now
> KDECMakeSettings.cmake:323
> +COMMAND ruby "${CMAKE_BINARY_DIR}/releaseme/fetchpo.rb" --origin
>
apol added a reviewer: ilic.
apol added a comment.
Ping?
REPOSITORY
R240 Extra CMake Modules
REVISION DETAIL
https://phabricator.kde.org/D5143
To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid, ilic
Cc: sitter
apol marked an inline comment as done.
REPOSITORY
R240 Extra CMake Modules
REVISION DETAIL
https://phabricator.kde.org/D5143
To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid
Cc: sitter
apol updated this revision to Diff 13006.
apol added a comment.
Few improvements
Use regex to figure out the repo name
Improve comment when fetching translations indicating what it's fetching
Correct arguments to add_custom_target
REPOSITORY
R240 Extra CMake Modules
CHANGES SINCE
apol added a comment.
> This only works if you used kde:okular but not
git://anongit.kde.org/okular.git, no?
Addressed on the next interation.
REPOSITORY
R240 Extra CMake Modules
REVISION DETAIL
https://phabricator.kde.org/D5143
To: apol, #frameworks, #build_system, kfunk,
sitter added a comment.
Oh! OTOH, given fetch-translations pulls it into bin_dir while normally we'd
have it in src_dir this may well be awkward in implementation. e.g.
https://phabricator.kde.org/D5197 effectively ought to be
ki18n_install("${CMAKE_SOURCE_DIR}/po")
sitter added a comment.
In https://phabricator.kde.org/D5143#97985, @apol wrote:
> In https://phabricator.kde.org/D5143#97847, @aacid wrote:
>
> > That creates a dependency for ki18n for frameworks that only use .ts
files, no?
>
>
> One thing we can do is remove the
apol added a comment.
In https://phabricator.kde.org/D5143#97847, @aacid wrote:
> That creates a dependency for ki18n for frameworks that only use .ts files,
no?
One thing we can do is remove the ki18n_install step from here and move it to
each project like this, much like we're
apol updated this revision to Diff 12868.
apol added a comment.
Just do the translation fetching
Including the po should be done by the project itself
REPOSITORY
R240 Extra CMake Modules
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D5143?vs=12776=12868
BRANCH
master
sitter added a comment.
The way I understand it the use case here is during development, so I am not
sure the ki18n circular dep is particularly concerning. Avoiding it seems
fairly difficult as find_package needs to happen at configure-time,
fetch-translations at build-time, and so we only
apol added a comment.
In https://phabricator.kde.org/D5143#97847, @aacid wrote:
> That creates a dependency for ki18n for frameworks that only use .ts files,
no?
It's not really a dependency. It just won't do much over there. AFAIK.
FWIW, I thought this for KI18n framework
aacid added a comment.
That creates a dependency for ki18n for frameworks that only use .ts files,
no?
REPOSITORY
R240 Extra CMake Modules
REVISION DETAIL
https://phabricator.kde.org/D5143
To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid
Cc: sitter
apol added a comment.
In https://phabricator.kde.org/D5143#97840, @aacid wrote:
> why find_package(KF5I18n) ?
Because `ki18n_install(po)`
REPOSITORY
R240 Extra CMake Modules
REVISION DETAIL
https://phabricator.kde.org/D5143
To: apol, #frameworks, #build_system, kfunk,
aacid added a comment.
why find_package(KF5I18n) ?
REPOSITORY
R240 Extra CMake Modules
REVISION DETAIL
https://phabricator.kde.org/D5143
To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid
Cc: sitter
apol added a comment.
See https://phabricator.kde.org/D5167, with this all mentioned issues should
be fixed.
See the workaround I had to introduce in 287. Before doing any changes, I'd
like some thoughts on how should we deal with projects that use Qt translations
system.
REPOSITORY
apol updated this revision to Diff 12776.
apol added a comment.
prefer trunk translations branch
REPOSITORY
R240 Extra CMake Modules
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D5143?vs=12732=12776
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D5143
AFFECTED
sitter added a comment.
A couple of things I am not sure about
- defaults to stable translations, I'd say it should default to trunk. e.g.
playground and kdereview don't even have stable
- considering one has to enable this feature in the cmakecache I think it
should be made a
apol updated this revision to Diff 12732.
apol added a comment.
Add documentation
REPOSITORY
R240 Extra CMake Modules
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D5143?vs=12711=12732
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D5143
AFFECTED FILES
apol marked 2 inline comments as done.
apol added a comment.
will send another review with the documentation
INLINE COMMENTS
> sitter wrote in KDECMakeSettings.cmake:302
> I wonder if we shouldn't use `ExternalProject` here. The advantage being that
> cmake would manage the clone and make
sitter added inline comments.
INLINE COMMENTS
> KDECMakeSettings.cmake:74
> # - ``APPLE_SUPPRESS_X11_WARNING`` option since 5.14.0
>
>
> #=
Needs documentation for the l10n awesomeness.
>
apol created this revision.
Restricted Application added projects: Frameworks, Build System.
REVISION SUMMARY
Makes it possible to fetch translations when building the project. To do so
it fetches kde:releaseme and uses the new fetchpo.rb program to download the
translations into the build
27 matches
Mail list logo
