DevHeads.net

AtCore on KDE Review

Yay, now we are on track again. This will be the right thread for the
review.
I will address here the issues that aacid and Luigui opened on the previous
thread and allow the others AtCore devels answer it.

-> aacid

"Partially, i personally still think it'd be better if you move the
PrinterState AXIS and MeasuramentUnits enums inside AtCore (or make them
C++11
"enum class").

Also note how PrinterState AXIS MeasuramentUnits is not consistent naming
"

For that, I think this is the diff: <a href="https://phabricator.kde.org/D6363" title="https://phabricator.kde.org/D6363">https://phabricator.kde.org/D6363</a>

-> Luigui
"In addition to Albert's comment, I noticed now (still going through the
backlog after vacation) that atcore use tr() for messages, but there is no
Messages.sh file to extract the strings (which should be called atcore_qt,
check the similar files in step or marble or in tier1 frameworks)."‚Äč

Cheers,

Comments

Re: AtCore on KDE Review

By Lays Rodrigues at 07/05/2017 - 07:23

Hi guys of kde-core.

Any new review of AtCore? =D

Cheers,

On Fri, Jun 23, 2017 at 11:19 PM, Lays Rodrigues <

Re: AtCore on KDE Review

By Luigi Toscano at 07/06/2017 - 07:03

Lays Rodrigues ha scritto:
I think that there were questions open on your side. Did you address the two
issues, namely:

This seems to be merged; Albert, does it address your concern?

I don't think that this has been addressed

Re: AtCore on KDE Review

By Tomaz Canabrava at 07/07/2017 - 08:02

On Thu, Jul 6, 2017 at 2:03 PM, Luigi Toscano <luigi. ... at tiscali dot it>
wrote:

It was not, and it's my fault.
I was looking on marble sources but I didn't understood how the Messages.sh
works,
I'm reading and will update that as soon as I can.

Re: AtCore on KDE Review

By Luigi Toscano at 07/07/2017 - 08:05

On Friday, 7 July 2017 15:02:21 CEST you wrote:
Or tier1 Frameworks.

<a href="https://cgit.kde.org/kuserfeedback.git/tree/src/provider/Messages.sh" title="https://cgit.kde.org/kuserfeedback.git/tree/src/provider/Messages.sh">https://cgit.kde.org/kuserfeedback.git/tree/src/provider/Messages.sh</a>

To test it:

<a href="http://tsdgeos.blogspot.cz/2010/08/how-to-run-messagessh-file-to-create.html" title="http://tsdgeos.blogspot.cz/2010/08/how-to-run-messagessh-file-to-create.html">http://tsdgeos.blogspot.cz/2010/08/how-to-run-messagessh-file-to-create....</a>

Ciao

Re: AtCore on KDE Review

By Albert Astals Cid at 07/10/2017 - 17:07

El divendres, 7 de juliol de 2017, a les 15:05:38 CEST, Luigi Toscano va
escriure:
Or the actual wiki page that should contain kind of updated contents hopefully

<a href="https://techbase.kde.org/Development/Tutorials/Localization/" title="https://techbase.kde.org/Development/Tutorials/Localization/">https://techbase.kde.org/Development/Tutorials/Localization/</a>
i18n_Build_Systems#Writing_a_Messages.sh_script

Cheers,
Albert

Re: AtCore on KDE Review

By Lays Rodrigues at 07/12/2017 - 07:19

I don't see the importance to add a translation to AtCore Test Client since
Atelier should be the main client (in one month or two).
The test client is only for testing.
Cheers,

Re: AtCore on KDE Review

By Luigi Toscano at 07/12/2017 - 07:30

On Wednesday, 12 July 2017 14:19:29 CEST you wrote:
There are ::tr calls also in
src/gcodecommands.cpp:

and one inside src/atcore.cpp, but it's in a qCDebug and should most likely be
removed.

Re: AtCore on KDE Review

By Lays Rodrigues at 07/12/2017 - 16:36

On Wed, Jul 12, 2017 at 9:30 AM, Luigi Toscano <luigi. ... at tiscali dot it>
wrote:

Re: AtCore on KDE Review

By Lays Rodrigues at 07/13/2017 - 18:44

Well, choosed to maintain the translation.
Diff with the script merged:
<a href="https://phabricator.kde.org/D6692" title="https://phabricator.kde.org/D6692">https://phabricator.kde.org/D6692</a>

Thanks Luigui!

On Wed, Jul 12, 2017 at 6:36 PM, Lays Rodrigues <

Re: AtCore on KDE Review

By Luigi Toscano at 07/16/2017 - 08:50

Lays Rodrigues ha scritto:
That's solved, but there is another issue that I noticed now: atcore uses the
same pattern as Frameworks libraries, and this should be fixed.
For example, see the following files installed:

<prefix>/lib/x86_64-linux-gnu/cmake/KF5AtCore/KF5AtCoreConfig.cmake
...
<prefix>/include/KF5/atcore_version.h
<prefix>/lib/x86_64-linux-gnu/libKF5AtCore.so.16.08.0
<prefix>/lib/x86_64-linux-gnu/libKF5AtCore.so.1
<prefix>/lib/x86_64-linux-gnu/libKF5AtCore.so

The prefix KF5 should go away.

Ciao

Re: AtCore on KDE Review

By sithlord48 at 07/16/2017 - 10:27

Atcore is a kde framework library So we used the KF5 prefix. If this is
still incorrect then I can remove it

Re: AtCore on KDE Review

By Luigi Toscano at 07/16/2017 - 10:31

sithlord48 ha scritto:
I think that there is a misunderstanding: AtCore is a library based on Qt and
ECM. Even if it used some libraries from Frameworks, it would not make it part
of "KDE Frameworks", but a library *based on* KDE Frameworks.
The only libraries which are part of KDE Frameworks are, well, the ones
shipped with KDE Frameworks:
<a href="https://api.kde.org/frameworks/index.html" title="https://api.kde.org/frameworks/index.html">https://api.kde.org/frameworks/index.html</a>
<a href="https://download.kde.org/stable/frameworks/5.36/" title="https://download.kde.org/stable/frameworks/5.36/">https://download.kde.org/stable/frameworks/5.36/</a>

Re: AtCore on KDE Review

By sithlord48 at 07/16/2017 - 10:33

Thank you for clarifying that as I did misunderstand. I should hopefully
have a patch for this today . I will tag you for review.

sithlord48 ha scritto:
I think that there is a misunderstanding: AtCore is a library based on Qt
and
ECM. Even if it used some libraries from Frameworks, it would not make it
part
of "KDE Frameworks", but a library *based on* KDE Frameworks.
The only libraries which are part of KDE Frameworks are, well, the ones
shipped with KDE Frameworks:
<a href="https://api.kde.org/frameworks/index.html" title="https://api.kde.org/frameworks/index.html">https://api.kde.org/frameworks/index.html</a>
<a href="https://download.kde.org/stable/frameworks/5.36/" title="https://download.kde.org/stable/frameworks/5.36/">https://download.kde.org/stable/frameworks/5.36/</a>