Re: KDEReview for kio-s3

[Elvis Angelaccio]

On 20/09/22 23:55, Albert Astals Cid wrote:

El dimarts, 20 de setembre de 2022, a les 0:17:52 (CEST), Elvis Angelaccio va
escriure:

Hi all,
I think kio-s3 [1] is now ready to go through the KDEReview process and
get a first release.

dfaure and sitter did a first code review in the original MR against
kio-extras [1]. The code hasn't changed much since then, mostly the port
to the new WorkerBase interface.

Please have a look. Thanks :)


Small enough, i didn't find anything to mention :)

Cheers,
   Albert


Thanks Albert.

I've now updated the projectpath on the metadata repo. Will soon do a 
first beta release.


Cheers,
Elvis



P.S.
You might notice that the gitlab CI is currently broken, I have already
filed a sysadmin ticket trying to sort it out.

[1]: https://invent.kde.org/network/kio-s3
[2]: https://invent.kde.org/network/kio-extras/-/merge_requests/35


Cheers,
Elvis

Re: KDEReview for kio-s3

[Albert Astals Cid]

El dimarts, 20 de setembre de 2022, a les 0:17:52 (CEST), Elvis Angelaccio va 
escriure:
> Hi all,
> I think kio-s3 [1] is now ready to go through the KDEReview process and
> get a first release.
> 
> dfaure and sitter did a first code review in the original MR against
> kio-extras [1]. The code hasn't changed much since then, mostly the port
> to the new WorkerBase interface.
> 
> Please have a look. Thanks :)

Small enough, i didn't find anything to mention :)

Cheers,
  Albert

> 
> P.S.
> You might notice that the gitlab CI is currently broken, I have already
> filed a sysadmin ticket trying to sort it out.
> 
> [1]: https://invent.kde.org/network/kio-s3
> [2]: https://invent.kde.org/network/kio-extras/-/merge_requests/35
> 
> 
> Cheers,
> Elvis

Re: kdereview Telly Skout

[Albert Astals Cid]

El dimarts, 10 de maig de 2022, a les 20:02:07 (CEST), Plata va escriure:
> Hi,
> 
> 
> thanks for the feedback.

Please keep kde-core-devel in CC

> 
> Yes, I've generally planned to support multiple sources (not only TV 
> Spielfilm). Do you have a particular one you'd like to have?

Not particularly, no.

> I've tried to add KCrash 
> (https://invent.kde.org/plasma-mobile/telly-skout/-/merge_requests/7) 
> but it fails the CI. Currently, I don't know why.

You need to add kcrash to 
https://invent.kde.org/plasma-mobile/telly-skout/-/blob/master/.kde-ci.yml

> 
> I'm investigating the crash. I've opened 
> https://invent.kde.org/plasma-mobile/telly-skout/-/issues/12 to track it 
> and I can reproduce it but couldn't find the error yet.

Sent https://invent.kde.org/plasma-mobile/telly-skout/-/merge_requests/8 your 
way.

Cheers,
  Albert

> 
> 
> Am 09.05.22 um 23:29 schrieb Albert Astals Cid:
> > El dilluns, 9 de maig de 2022, a les 18:38:05 (CEST), Plata va escriure:
> >> Hi,
> >>
> >>
> >> after my TV guide application Telly Skout has been moved to
> >> https://invent.kde.org/plasma-mobile/telly-skout, I'm asking for
> >> kdereview (according to
> >> https://community.kde.org/Policies/Application_Lifecycle#kdereview).
> > Are there plans to support anything els than Germany?
> >
> > Please add
> >KCrash::initialize
> > The thing just crashed on me and didn't give me the expected "i have 
> > crashed" dialog.
> >
> > Valgrind trace of the crash https://ghostbin.com/WP3X8
> >
> > Cheers,
> >Albert
> >
> >> Please open issues in GitLab if you find problems/possibilities for
> >> improvement.
> >>
> >>
> >> Thanks
> >>
> >> Plata
> >>
> >>
> >
> >
> >
> 

Re: kdereview Telly Skout

[Albert Astals Cid]

El dilluns, 9 de maig de 2022, a les 18:38:05 (CEST), Plata va escriure:
> Hi,
> 
> 
> after my TV guide application Telly Skout has been moved to 
> https://invent.kde.org/plasma-mobile/telly-skout, I'm asking for 
> kdereview (according to 
> https://community.kde.org/Policies/Application_Lifecycle#kdereview).

Are there plans to support anything els than Germany?

Please add 
  KCrash::initialize
The thing just crashed on me and didn't give me the expected "i have crashed" 
dialog.

Valgrind trace of the crash https://ghostbin.com/WP3X8

Cheers,
  Albert

> 
> Please open issues in GitLab if you find problems/possibilities for 
> improvement.
> 
> 
> Thanks
> 
> Plata
> 
> 

[DONE] Re: KDEREVIEW: Proposing utilities/markdownpart to become community/release-service-managed

[Friedrich W. H. Kossebau]

Hi,

Am Samstag, 15. August 2020, 19:20:53 CEST schrieb Friedrich W. H. Kossebau:
> what is markdownpart you ask? A KParts plugin allowing to view markdown
> documents rendered e.g. in Kate's preview plugin, Ark, Krusader or
> Konqueror, being mainly a wrapper around QTextDocument::setMarkdown &
> QTextBrowser. See here it being used with Kate's preview plugin:
> https://cdn.kde.org/screenshots/markdownpart/markdownpart.png
> Note (edit: updated): Qt 5.14 minimum needed
> 
> I would like to propose markdownpart for a move into community maintenance
> mode and becoming part of release service managed projects starting with RS
> 20.12. It would match graphics/svgpart in the mode (whose code mode BTW is
> aöso similar, mainly a wrapper around QSvgRenderer & QGraphicsView).

Am Montag, 24. August 2020, 00:30:13 CEST schrieb Friedrich W. H. Kossebau:
> Small plan change here:
> given 20.12 is quite some weeks away still, markdownpart should move post-
> kdereview first into extragear, for one or two releases with the initial
> feedback added and especially translations, and only then enter release
> service latest in November before the repo freeze.

Thanks everyone for their review & comments.

Without any open objections or todos, I am now executing the sketchecd plan:
* move markdownpart into extragear/utils today
* do official string freeze on trunk today
* do a 0.1.1 release on Monday, September 21st from trunk
  (so translators have 3 weeks occasion to add support for their locale)
* move into release service set before November
  (waiting a bit after 0.1.1 in chance bugs are reported which should be fixed
  in a release before 20.12)

Cheers
Friedrich

Re: KDEREVIEW: Proposing utilities/markdownpart to become community/release-service-managed

[Friedrich W. H. Kossebau]

Am Samstag, 15. August 2020, 19:20:53 CEST schrieb Friedrich W. H. Kossebau:
> Note: for now Qt 5.15-only, 5.14 possible but untested.

BTW, thanks to feedback the min dependencies are now lowered to Qt 5.14 & KF 
5.66.

> I would like to propose markdownpart for a move into community maintenance
> mode and becoming part of release service managed projects starting with RS
> 20.12. It would match graphics/svgpart in the mode (whose code mode BTW is
> aöso similar, mainly a wrapper around QSvgRenderer & QGraphicsView).

Small plan change here:
given 20.12 is quite some weeks away still, markdownpart should move post-
kdereview first into extragear, for one or two releases with the initial 
feedback added and especially translations, and only then enter release 
service latest in November before the repo freeze.

Makes sense?

Otherwise thanks for review & feedback so far. Seems there have no obstacles 
come up so far, so upcoming Saturday markdownpart can then leave the current 
stage, and would go to extragear/utils, right into preparation of first full 
release.

Cheers
Friedrich

Re: KDEREVIEW: Proposing utilities/markdownpart to become community/release-service-managed

[Friedrich W. H. Kossebau]

Am Freitag, 21. August 2020, 23:34:03 CEST schrieb Albert Astals Cid:
> El dilluns, 17 d’agost de 2020, a les 23:04:44 CEST, Friedrich W. H. 
Kossebau va escriure:
> > But not my call, after all I offer this to KDE community for adoption, sou
> > your choice.
> 
> I'm a bit concerned about this being "abandonware" from birth, but seems
> there's relative interest from the community to collectively maintain it so
> not going to complain if this ends up in release service.

Thanks for statement, Albert.

Cheers
Friedrich

Re: KDEReview for Kontrast

[Carl Schwan]

Le jeudi, juillet 30, 2020 11:16 AM, Carl Schwan  a écrit :

> Hi,
> I would like to move Kontrast, a contrast checker application, to KDEReview. 
> Kontrast can check if two colors pass the WCAG 2.0 specification and save 
> some user's favorite color combinations.
>
> Some screenshots of the application and a design review from the VSG is 
> available here: https://invent.kde.org/accessibility/kontrast/-/issues/1
>
> From a code point of view, the application is very simple, but I still would 
> appreciate a general code review on it (it's my first Qt app written from 
> scratch). The code is available here: 
> https://invent.kde.org/accessibility/kontrast
>
> I don't plan to add new features and would like after the KDEReview, to 
> release a first version of the application, and then move it to the release 
> service so that the application gets regularly translations improvement.
>
> Thanks
> Carl

Hi again,

It is now more than 14 days that Kontrast is in KDEReview and I think I fixed 
all the points raised here and in IRC. I would like to release the first 
version somewhere next week and possibly a minor version 2 or 3 weeks later for 
an additional translation update and possible bug fixes. So I will move to 
extragear this evening (CEST) if nobody object to that.

Cheers,
Carl

Re: KDEREVIEW: Proposing utilities/markdownpart to become community/release-service-managed

[Albert Astals Cid]

El dilluns, 17 d’agost de 2020, a les 23:04:44 CEST, Friedrich W. H. Kossebau 
va escriure:
> But not my call, after all I offer this to KDE community for adoption, sou 
> your choice.

I'm a bit concerned about this being "abandonware" from birth, but seems 
there's relative interest from the community to collectively maintain it so not 
going to complain if this ends up in release service.

Cheers,
  Albert

> 
> Cheers
> Friedrich
> 
> 
> 

Re: KDEREVIEW: Proposing utilities/markdownpart to become community/release-service-managed

[Kevin Kofler]

Ivan Čukić wrote:
> markdownpartfactory.cpp:45
> 
> Personal preference - use `auto` when you have `= new Something(:::)` on
> the right - no need for `Something` to be written twice:
> Something* p = new Something(...)
> 
> The variable part can even go away - just return new MarkdownPart.
> 
> Similar lines exist in markdownpart.cpp, though there you use auto almost
> in all these cases.

IMHO, this just makes the code harder to read (as most uses of "auto").

Kevin Kofler

Re: KDEREVIEW: Proposing utilities/markdownpart to become community/release-service-managed

[Elvis Angelaccio]

On 15/08/20 19:20, Friedrich W. H. Kossebau wrote:
> Hi,
> 
> what is markdownpart you ask? A KParts plugin allowing to view markdown 
> documents rendered e.g. in Kate's preview plugin, Ark, Krusader or Konqueror, 
> being mainly a wrapper around QTextDocument::setMarkdown & QTextBrowser.
> See here it being used with Kate's preview plugin:
> https://cdn.kde.org/screenshots/markdownpart/markdownpart.png
> Note: for now Qt 5.15-only, 5.14 possible but untested.
> 
> I would like to propose markdownpart for a move into community maintenance 
> mode and becoming part of release service 

Hi Friedrich,

why not merge this plugin into kate instead?

> 
> Cheers
> Friedrich
> 
> 

Cheers,
Elvis

Re: KDEREVIEW: Proposing utilities/markdownpart to become community/release-service-managed

[Elvis Angelaccio]

On 17/08/20 23:04, Friedrich W. H. Kossebau wrote:
> Hi Elvis,
> 
> Am Montag, 17. August 2020, 22:43:37 CEST schrieb Elvis Angelaccio:
>> On 15/08/20 19:20, Friedrich W. H. Kossebau wrote:
>>> Hi,
>>>
>>> what is markdownpart you ask? A KParts plugin allowing to view markdown
>>> documents rendered e.g. in Kate's preview plugin, Ark, Krusader or
>>> Konqueror, being mainly a wrapper around QTextDocument::setMarkdown &
>>> QTextBrowser. See here it being used with Kate's preview plugin:
>>> https://cdn.kde.org/screenshots/markdownpart/markdownpart.png
>>> Note: for now Qt 5.15-only, 5.14 possible but untested.
>>>
>>> I would like to propose markdownpart for a move into community maintenance
>>> mode and becoming part of release service
>>
>> Hi Friedrich,
>>
>> why not merge this plugin into kate instead?
> 
> Let me answer with another question:
> why with Kate and not with Ark or with Krusader or with Konqueror or any 
> other 
> potential KParts plugin user where Markdown viewing often is useful?
> 
> Bundling with Kate would be a rather random choice IMHO. And
> a) make it challenging for packagers to split out an independent packager for 
> the markdown kpart with all the files belonging to it
> b) make it harder for anyone interested in hacking on it, as they would have 
> to also care about the whole Kate repo and its complete build system, when 
> they just want to improve the markdown viewer e.g. for Ark.
> 
> But not my call, after all I offer this to KDE community for adoption, sou 
> your choice.

My bad, I didn't realize this was a KPart plugin. The Kate screenshot
you linked got me confused :p

Then yes, a stand-alone package makes sense indeed.

> 
> Cheers
> Friedrich
> 
> 

Cheers,
Elvis

Re: KDEREVIEW: Proposing utilities/markdownpart to become community/release-service-managed

[Friedrich W. H. Kossebau]

Hi Elvis,

Am Montag, 17. August 2020, 22:43:37 CEST schrieb Elvis Angelaccio:
> On 15/08/20 19:20, Friedrich W. H. Kossebau wrote:
> > Hi,
> > 
> > what is markdownpart you ask? A KParts plugin allowing to view markdown
> > documents rendered e.g. in Kate's preview plugin, Ark, Krusader or
> > Konqueror, being mainly a wrapper around QTextDocument::setMarkdown &
> > QTextBrowser. See here it being used with Kate's preview plugin:
> > https://cdn.kde.org/screenshots/markdownpart/markdownpart.png
> > Note: for now Qt 5.15-only, 5.14 possible but untested.
> > 
> > I would like to propose markdownpart for a move into community maintenance
> > mode and becoming part of release service
> 
> Hi Friedrich,
> 
> why not merge this plugin into kate instead?

Let me answer with another question:
why with Kate and not with Ark or with Krusader or with Konqueror or any other 
potential KParts plugin user where Markdown viewing often is useful?

Bundling with Kate would be a rather random choice IMHO. And
a) make it challenging for packagers to split out an independent packager for 
the markdown kpart with all the files belonging to it
b) make it harder for anyone interested in hacking on it, as they would have 
to also care about the whole Kate repo and its complete build system, when 
they just want to improve the markdown viewer e.g. for Ark.

But not my call, after all I offer this to KDE community for adoption, sou 
your choice.

Cheers
Friedrich

Re: KDEREVIEW: Proposing utilities/markdownpart to become community/release-service-managed

[Friedrich W. H. Kossebau]

Hi Ivan,

Am Sonntag, 16. August 2020, 17:00:23 CEST schrieb Ivan Čukić:
> Hi Friedrich,
> 
> Very nice, thanks for doing this!

Glad to see people liking it, so my time was more worth it :) 
And thank you for review.

> These are the few nit-picks (feel free to ignore them) I have:

Nitpicks are welcome with me :) Actually taking culprit in not having run any 
code checker before uploading, bad me. And trying to excuse that with "Was 
just a quick proof of feasibility for others to use" sounds awful typing it, 
darn, no way out, lesson learned.
Hm, I could blame the wiki page
https://community.kde.org/ReleasingSoftware#Sanity_Checklist
not yet having mentioned running sanity tools :)
Added that there for everyone's next time now.

> markdownview.cpp:55
> 
> The `hasSelection` variable makes the code look much more complex than it
> is. It can be removed and the last argument of the `Q_EMIT
> contextMenuRequested` call set to:
> !linkUrl.isValid() && hasSelection()

Yes, does look strange without context, will clean up. Though still as 
separate variable, so the parameter as passed in the emit call gets some 
semantic by the variable name.
Motivation was to keep the code similar to patterns used in KWebKitPart & 
WebEnginePart, to allow some more detailed context menu once possible. Current 
QTextDocument/QTextBrowser API though does not provide same level of detail, 
so resulting in that empty logic, which I can see how it confuses.

Adding a comment instead, so the next person extending things knows what to 
look at to keep things consistent.

> markdownpartfactory.cpp:45
> 
> Personal preference - use `auto` when you have `= new Something(:::)` on the
> right - no need for `Something` to be written twice:
> Something* p = new Something(...)
> 
> The variable part can even go away - just return new MarkdownPart.
> 
> Similar lines exist in markdownpart.cpp, though there you use auto almost in
> all these cases.

Not repeating type name is also my preference (though with pointer indicator * 
-> "auto*" here to make code more obvious). Excuse: this was the old code just 
copied from kmarkdownwebview I did not have to touch, so stayed away from 
messing with. Now you made me (will backport also to keep diff small).

Also removed temporary variable as proposed, development left-over.

> markdownbrowserextension.cpp:51
> 
> There is no point in having the `const bool forcesNewWindow = false`
> declared at the beginning of the function when it is used only in a single
> line at the end of the function:
> bargs.setForcesNewWindow(forcesNewWindow);
> 
> Replacing this with false is IMO more readable as you don't need to scroll
> up to check what the value of forcesNewWindow is:
> bargs.setForcesNewWindow(false);

Same reasoning as with hasSelection of the context menu. And will do same 
resolution, making code simple, but pointing to similar code for consistency 
in handling on potential future changes.

Should be all dealt with now in latest master :)

Cheers
Friedrich

Re: KDEREVIEW: Proposing utilities/markdownpart to become community/release-service-managed

[Ivan Čukić]

Hi Friedrich,

Very nice, thanks for doing this!

These are the few nit-picks (feel free to ignore them) I have:

markdownview.cpp:55

The `hasSelection` variable makes the code look much more complex than it is.
It can be removed and the last argument of the `Q_EMIT contextMenuRequested` 
call set to:
!linkUrl.isValid() && hasSelection()


markdownpartfactory.cpp:45

Personal preference - use `auto` when you have `= new Something(:::)` on the 
right - no need for `Something` to be written twice:
Something* p = new Something(...)

The variable part can even go away - just return new MarkdownPart.

Similar lines exist in markdownpart.cpp, though there you use auto almost in 
all these cases.


markdownbrowserextension.cpp:51

There is no point in having the `const bool forcesNewWindow = false` declared 
at the beginning of the function when it is used only in a single line at the 
end of the function:
bargs.setForcesNewWindow(forcesNewWindow);

Replacing this with false is IMO more readable as you don't need to scroll up 
to check what the value of forcesNewWindow is:
bargs.setForcesNewWindow(false);


Cheers,
Ivan


-- 
dr Ivan Čukić
i...@cukic.co, https://cukic.co/
gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232  07AE 01C6 CE2B FF04 1C12

Re: KDEREVIEW: Proposing utilities/markdownpart to become community/release-service-managed

[Friedrich W. H. Kossebau]

Am Sonntag, 16. August 2020, 00:03:53 CEST schrieb David Faure:
> The code looks fine to me.

Thanks for review.
 
> The only thing I saw was case-insensitive comparison of the result of
> QUrl::scheme() which is unnecessary, it always returns lowercase.

Ah, was not aware (and missed the inconsistency with other checks of the 
scheme). 
Going to fix the same also in KWebKitPart, WebEnginePart, and 
KMarkdownWebView, which is the copy trace of that code snippet from what 
I can tell :)
And actually discovered my code had some inverted logic flaw as well, also 
fixed.

> Glad to see KParts still lives :-)

Living code fossils, still not ready for static display in museum :) 

Cheers
Friedrich

Re: KDEREVIEW: Proposing utilities/markdownpart to become community/release-service-managed

[David Faure]

On samedi 15 août 2020 19:20:53 CEST Friedrich W. H. Kossebau wrote:
> Hi,
> 
> what is markdownpart you ask? A KParts plugin allowing to view markdown
> documents rendered e.g. in Kate's preview plugin, Ark, Krusader or
> Konqueror, being mainly a wrapper around QTextDocument::setMarkdown &
> QTextBrowser. See here it being used with Kate's preview plugin:
> https://cdn.kde.org/screenshots/markdownpart/markdownpart.png
> Note: for now Qt 5.15-only, 5.14 possible but untested.
> 
> I would like to propose markdownpart for a move into community maintenance
> mode and becoming part of release service managed projects starting with RS
> 20.12. It would match graphics/svgpart in the mode (whose code mode BTW is
> aöso similar, mainly a wrapper around QSvgRenderer & QGraphicsView).
> 
> The code lives at https://invent.kde.org/utilities/markdownpart since
> yesterday.
> I consider the project stable & done already though, given there are no more
> features I want myself and given it is based on existing code: mainly a
> copy of KMarkdownWebView, which itself was inspired/driven by e.g. KWebKit/
> KWebEngine KParts code.
> Some small glitches are with the search tool and incremental search
> sometimes, but that might be a bug in QTextDocument/QTextBrowser.
> 
> I have written this plugin mainly because "I could :P" and some people might
> like it, less because I want to use this myself everyday (personally
> dislike Markdown). So I would be interested to pass maintenance over into
> community domain, or interested individuals if there are. Otherwise it
> would be a candidate for the attic.
> 
> Today I released a first 0.1.0 version to make it spread and find users who
> fancy it:
> https://mail.kde.org/pipermail/kde-announce-apps/2020-August/005597.html
> 
> Try yourself by building and installing with "kdesrc-build markdownpart" and
> picking "Markdown View (markdownpart)" as preferred embedding plugin for
> "text/markdown" in System Settings' File Associations submodule (part of
> "Applications").
> 
> 
> So I hereby move markdownpart into kdereview mode.
> 
> Please give the code some eyeball times and comment for good and bad or
> missing stuff and whether you agree if this should become part of KDE
> community-managed set of software. Also hoping to see someone offering
> themselves to adopt this project as caring maintainer.
> 
> 
> BACKGROUND
> 
> You might know the KParts plugin from KMarkdownWebView for the same purpose
> (https://invent.kde.org/utilities/kmarkdownwebview).
> 
> It had been written 3 years ago as quick solution, with an important
> statement in the README:
> "The software should serve as intermediate solution until some native
> Qt-based implementation is done."
> 
> While there has been some native variant present meanwhile via the Okular
> Markdown support for some time, and available via the Okular KParts plugin,
> the paged display of Okular might not meet the desire of some to see the
> Markdown document rendered as single adaptive page.
> 
> With QTextDocument having gotten some native markdown-parsing support in Qt
> 5.14, some recent discussion about whether to include KMarkdownWebView (and
> the QWeb* dependencies it brings) into Kate app bundles for the preview
> feature triggered me to give it a try to write a QTextDocument-based
> variant. Which turned out to be simple to do in an evening, and so here we
> are.

The code looks fine to me.

The only thing I saw was case-insensitive comparison of the result of 
QUrl::scheme() which is unnecessary, it always returns lowercase.

Glad to see KParts still lives :-)

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

Re: KDEReview for Kontrast

[Michael Reeves]

The odd naming is a flatpac requirement if you want the icon auto
extracted. Otherwise flatpac will have to be told explicitly to rename the
icon.

On Tue, Aug 11, 2020, 3:38 AM Christophe Giboudeaux 
wrote:

> On lundi 3 août 2020 23:12:25 CEST Albert Astals Cid wrote:
> > > I added the icon and I hope I installed it to the correct location:
> > >
> https://invent.kde.org/accessibility/kontrast/-/commit/8a008c1387c0234d5e
> > > 1d537bdd331007d7b1ff07. It was already stored in breeze-icons but I
> guess
> > > it is better to also have the app icon in the application so that it is
> > > displayed on other DEs.
> > I'm 93% sure the file should be kontrast.svg given you're doing
> >
> QApplication::setWindowIcon(QIcon::fromTheme(QStringLiteral("kontrast")));
>
> Then the desktop file would have to be changed :)
> It contains Icon=org.kde.kontrast
>
>
>
>
>

Re: KDEReview for Kontrast

[Carl Schwan]

Le lundi, août 3, 2020 11:12 PM, Albert Astals Cid  a écrit :

> El dilluns, 3 d’agost de 2020, a les 4:26:25 CEST, Carl Schwan va escriure:
>
> > Le dimanche, août 2, 2020 6:20 PM, Albert Astals Cid aa...@kde.org a écrit :
> >
> > > El dijous, 30 de juliol de 2020, a les 11:16:25 CEST, Carl Schwan va 
> > > escriure:
> > >
> > > > Hi,
> > > > I would like to move Kontrast, a contrast checker application, to 
> > > > KDEReview Kontrast can check if two colors pass the WCAG 2.0 
> > > > specification and save some user's favorite color combinations.
> > > > Some screenshots of the application and a design review from the VSG is 
> > > > available here: https://invent.kde.org/accessibility/kontrast/-/issues/1
> > > > From a code point of view, the application is very simple, but I still 
> > > > would appreciate a general code review on it (it's my first Qt app 
> > > > written from scratch). The code is available here: 
> > > > https://invent.kde.org/accessibility/kontrast
> > > > I don't plan to add new features and would like after the KDEReview, to 
> > > > release a first version of the application, and then move it to the 
> > > > release service so that the application gets regularly translations 
> > > > improvement.
> >
> > Hi Albert,
> > Thanks a lot for the review
> >
> > > You don't have an icon, which is not optimal [actually i see you have an 
> > > icon in invent.k.o so the hard part of drawing it seems to be done :)]
> >
> > I added the icon and I hope I installed it to the correct location: 
> > https://invent.kde.org/accessibility/kontrast/-/commit/8a008c1387c0234d5e1d537bdd331007d7b1ff07.
> >  It was already stored in breeze-icons but I guess it is better to also 
> > have the app icon in the application so that it is displayed on other DEs.
>
> I'm 93% sure the file should be kontrast.svg given you're doing
> QApplication::setWindowIcon(QIcon::fromTheme(QStringLiteral("kontrast")));

I updated my setWindowIcon call now and the icon now works.

>
> > > The # of the colors is cut for me https://i.imgur.com/1GC2sEU.png

I fixed this behavior of the TextField and also added a maximumLength attribute
so that the text can't be more than 7 characters long.

I also now make sure that the text color of the field is always visible.

https://invent.kde.org/accessibility/kontrast/-/commit/2d235189603c87ead8d03cc0bef9d1933716552d

> > > Missing i18n:
> > > ./src/contents/ui/MainPage.qml:28: title: "Please choose a color"
> >
> > Fixed now
> >
> > > Would be great if you had the typical --help --author, etc.
> > > See QCommandLineParser and KAboutData::setupCommandLine

I now also added the KAboutData::setupCommandLine call and --help and --author 
work.

> > > Would a documentation of the ranges make sense?
> > > i.e. something that has the ranges and the descriptions you put for each 
> > > of the ranges in one place? Something like a "Help" page.
> >
> > Great ideas, I put them on my TODO list. 
> > https://invent.kde.org/accessibility/kontrast/-/issues

There is now a basic help page adding information about the contrast range:
https://invent.kde.org/accessibility/kontrast/-/merge_requests/4

> >
> > > Could only test part of the app since you're requiring unreleased 
> > > Kirigami 2.14
> > > Which probably means your
> > > set(KF5_MIN_VERSION "5.70.0")
> > > should be changed to
> > > set(KF5_MIN_VERSION "5.73.0")
> >
> > I have now changed the kirigami dependency to require an older Kirigami 
> > version, since
> > I wasn't using any new Kirigami feature anyway.
>
> No you have not?
>
> ./src/contents/ui/FavoritePage.qml:8:import org.kde.kirigami 2.14 as Kirigami

Sorry it looks like I forgot to commit the change :(
https://invent.kde.org/accessibility/kontrast/-/commit/d73003f36d71ec036a7d612eb3487ea3801bd6c1
>
> > But I would still recommend using Kontrast
> > with the latest Kirigami version since the new version comes with a few 
> > Accessibility
> > improvements ;)
> >
> > > Out of curiosity any reason you decided to go with
> > > auto SavedColorModel::refresh() -> void
> > > instead of
> > > void SavedColorModel::refresh()
> > > ?
> >
> > This code was contributed by Carson Black and I have no strong preferences 
> > for the coding style
> > of the methods. I guess changing it back to the traditional style could 
> > make sense to follow
> > the general KDE coding style.
>
> No need, i was just curious about the waste of horizontal space :)
>
> Cheers,
> Albert
>
> > > Cheers,
> > > Albert
> > >
> > > > Thanks
> > > > Carl

Re: KDEReview for Kontrast

[Christophe Giboudeaux]

On lundi 3 août 2020 23:12:25 CEST Albert Astals Cid wrote:
> > I added the icon and I hope I installed it to the correct location:
> > https://invent.kde.org/accessibility/kontrast/-/commit/8a008c1387c0234d5e
> > 1d537bdd331007d7b1ff07. It was already stored in breeze-icons but I guess
> > it is better to also have the app icon in the application so that it is
> > displayed on other DEs.
> I'm 93% sure the file should be kontrast.svg given you're doing
> QApplication::setWindowIcon(QIcon::fromTheme(QStringLiteral("kontrast")));

Then the desktop file would have to be changed :)
It contains Icon=org.kde.kontrast

Re: KDEReview for Kontrast

[Albert Astals Cid]

El dilluns, 3 d’agost de 2020, a les 4:26:25 CEST, Carl Schwan va escriure:
> Le dimanche, août 2, 2020 6:20 PM, Albert Astals Cid  a écrit :
> 
> > El dijous, 30 de juliol de 2020, a les 11:16:25 CEST, Carl Schwan va 
> > escriure:
> >
> > > Hi,
> > > I would like to move Kontrast, a contrast checker application, to 
> > > KDEReview Kontrast can check if two colors pass the WCAG 2.0 
> > > specification and save some user's favorite color combinations.
> > > Some screenshots of the application and a design review from the VSG is 
> > > available here: https://invent.kde.org/accessibility/kontrast/-/issues/1
> > > From a code point of view, the application is very simple, but I still 
> > > would appreciate a general code review on it (it's my first Qt app 
> > > written from scratch). The code is available here: 
> > > https://invent.kde.org/accessibility/kontrast
> > > I don't plan to add new features and would like after the KDEReview, to 
> > > release a first version of the application, and then move it to the 
> > > release service so that the application gets regularly translations 
> > > improvement.
> 
> Hi Albert,
> 
> Thanks a lot for the review
> 
> > You don't have an icon, which is not optimal [actually i see you have an 
> > icon in invent.k.o so the hard part of drawing it seems to be done :)]
> 
> I added the icon and I hope I installed it to the correct location: 
> https://invent.kde.org/accessibility/kontrast/-/commit/8a008c1387c0234d5e1d537bdd331007d7b1ff07.
>  It was already stored in breeze-icons but I guess it is better to also have 
> the app icon in the application so that it is displayed on other DEs.

I'm 93% sure the file should be kontrast.svg given you're doing
QApplication::setWindowIcon(QIcon::fromTheme(QStringLiteral("kontrast")));

> >
> > The # of the colors is cut for me https://i.imgur.com/1GC2sEU.png
> >
> > Missing i18n:
> > ./src/contents/ui/MainPage.qml:28: title: "Please choose a color"
> 
> Fixed now
> 
> >
> > Would be great if you had the typical --help --author, etc.
> > See QCommandLineParser and KAboutData::setupCommandLine
> >
> > Would a documentation of the ranges make sense?
> > i.e. something that has the ranges and the descriptions you put for each of 
> > the ranges in one place? Something like a "Help" page.
> 
> Great ideas, I put them on my TODO list. 
> https://invent.kde.org/accessibility/kontrast/-/issues
> 
> >
> > Could only test part of the app since you're requiring unreleased Kirigami 
> > 2.14
> > Which probably means your
> > set(KF5_MIN_VERSION "5.70.0")
> > should be changed to
> > set(KF5_MIN_VERSION "5.73.0")
> 
> I have now changed the kirigami dependency to require an older Kirigami 
> version, since
> I wasn't using any new Kirigami feature anyway. 

No you have not?

./src/contents/ui/FavoritePage.qml:8:import org.kde.kirigami 2.14 as Kirigami


> But I would still recommend using Kontrast
> with the latest Kirigami version since the new version comes with a few 
> Accessibility
> improvements ;)
> 
> >
> > Out of curiosity any reason you decided to go with
> > auto SavedColorModel::refresh() -> void
> > instead of
> > void SavedColorModel::refresh()
> > ?
> 
> This code was contributed by Carson Black and I have no strong preferences 
> for the coding style
> of the methods. I guess changing it back to the traditional style could make 
> sense to follow
> the general KDE coding style.

No need, i was just curious about the waste of horizontal space :)

Cheers,
  Albert

> 
> 
> >
> > Cheers,
> > Albert
> >
> > > Thanks
> > > Carl
> 
> 

Re: KDEReview for Kontrast

[Carl Schwan]

Le dimanche, août 2, 2020 6:20 PM, Albert Astals Cid  a écrit :

> El dijous, 30 de juliol de 2020, a les 11:16:25 CEST, Carl Schwan va escriure:
>
> > Hi,
> > I would like to move Kontrast, a contrast checker application, to KDEReview 
> > Kontrast can check if two colors pass the WCAG 2.0 specification and save 
> > some user's favorite color combinations.
> > Some screenshots of the application and a design review from the VSG is 
> > available here: https://invent.kde.org/accessibility/kontrast/-/issues/1
> > From a code point of view, the application is very simple, but I still 
> > would appreciate a general code review on it (it's my first Qt app written 
> > from scratch). The code is available here: 
> > https://invent.kde.org/accessibility/kontrast
> > I don't plan to add new features and would like after the KDEReview, to 
> > release a first version of the application, and then move it to the release 
> > service so that the application gets regularly translations improvement.

Hi Albert,

Thanks a lot for the review

> You don't have an icon, which is not optimal [actually i see you have an icon 
> in invent.k.o so the hard part of drawing it seems to be done :)]

I added the icon and I hope I installed it to the correct location: 
https://invent.kde.org/accessibility/kontrast/-/commit/8a008c1387c0234d5e1d537bdd331007d7b1ff07.
 It was already stored in breeze-icons but I guess it is better to also have 
the app icon in the application so that it is displayed on other DEs.

>
> The # of the colors is cut for me https://i.imgur.com/1GC2sEU.png
>
> Missing i18n:
> ./src/contents/ui/MainPage.qml:28: title: "Please choose a color"

Fixed now

>
> Would be great if you had the typical --help --author, etc.
> See QCommandLineParser and KAboutData::setupCommandLine
>
> Would a documentation of the ranges make sense?
> i.e. something that has the ranges and the descriptions you put for each of 
> the ranges in one place? Something like a "Help" page.

Great ideas, I put them on my TODO list. 
https://invent.kde.org/accessibility/kontrast/-/issues

>
> Could only test part of the app since you're requiring unreleased Kirigami 
> 2.14
> Which probably means your
> set(KF5_MIN_VERSION "5.70.0")
> should be changed to
> set(KF5_MIN_VERSION "5.73.0")

I have now changed the kirigami dependency to require an older Kirigami 
version, since
I wasn't using any new Kirigami feature anyway. But I would still recommend 
using Kontrast
with the latest Kirigami version since the new version comes with a few 
Accessibility
improvements ;)

>
> Out of curiosity any reason you decided to go with
> auto SavedColorModel::refresh() -> void
> instead of
> void SavedColorModel::refresh()
> ?

This code was contributed by Carson Black and I have no strong preferences for 
the coding style
of the methods. I guess changing it back to the traditional style could make 
sense to follow
the general KDE coding style.


>
> Cheers,
> Albert
>
> > Thanks
> > Carl

Re: KDEReview for Kontrast

[Carl Schwan]

Le dimanche, août 2, 2020 2:58 PM, Nate Graham  a écrit :

> Hello Carl,
> This looks very useful! Overall I'd say the UI is good. One thing I find
> myself missing while playing around is the ability to test system
> colorschemes though. That would be a really nice addition.

I'm not sure this feature would make sense in Kontrast, but maybe this
functionality should be integrated into the new color scheme editor Noah
is building. The actual logic for calculating the contrast is very small.

What do you think?

>
> Nate
>
> On 7/30/20 3:16 AM, Carl Schwan wrote:
>
> > Hi,
> > I would like to move Kontrast, a contrast checker application, to 
> > KDEReview. Kontrast can check if two colors pass the WCAG 2.0 specification 
> > and save some user's favorite color combinations.
> > Some screenshots of the application and a design review from the VSG is 
> > available here: https://invent.kde.org/accessibility/kontrast/-/issues/1
> > From a code point of view, the application is very simple, but I still 
> > would appreciate a general code review on it (it's my first Qt app written 
> > from scratch). The code is available here: 
> > https://invent.kde.org/accessibility/kontrast
> > I don't plan to add new features and would like after the KDEReview, to 
> > release a first version of the application, and then move it to the release 
> > service so that the application gets regularly translations improvement.
> > Thanks
> > Carl

Re: KDEReview for Kontrast

[Carl Schwan]

Carl Schwan
https://carlschwan.eu

‐‐‐ Original Message ‐‐‐
Le dimanche, août 2, 2020 3:36 PM, Nicolas Fella  a écrit 
:

> Hi Carl,
>
> a couple of nitpicks, otherwise looks neat.
>
> -   your CMakeLists.txt does not specify a minimum Qt/KF5 version. Also
> ECM 0.0.8 is likely quite old and a bit optimistic
>
> -   Setting CMAKE_CXX_STANDARD to 11 is implicitly done by ECM, no need to
> do that manually. It also contradicts the
> target_compile_features(kontrast PRIVATE cxx_std_17) you set later
>
> -   You can save yourself the explicit call to qt5_add_resources by adding
> resources.qrc to the add_executable call. This is called AUTORCC in
> cmake and ECM enables it by default

It is probably worth changing this in the default Kirigami template in
KAppTemplete since this is where I took the code. Same for the 
CMAKE_CXX_STANDARD
declaration and the ECM 0.0.8.

>
> -   Instead of using QScopedPointer you should be able to just
>
> put Kontrast on the stack
>
> -   consider setting "isMenu: true" on your global drawer, that turns it
> into a hamburger menu on the desktop, which is more appropriate than a
> drawer

Thanks for the feedback, I think I have now fixed all the things you found.
>
> Cheers
>
> Nico
>
> On 30.07.20 11:16, Carl Schwan wrote:
>
>
> > Hi,
> > I would like to move Kontrast, a contrast checker application, to 
> > KDEReview. Kontrast can check if two colors pass the WCAG 2.0 specification 
> > and save some user's favorite color combinations.
> > Some screenshots of the application and a design review from the VSG is 
> > available here: https://invent.kde.org/accessibility/kontrast/-/issues/1
> > From a code point of view, the application is very simple, but I still 
> > would appreciate a general code review on it (it's my first Qt app written 
> > from scratch). The code is available here: 
> > https://invent.kde.org/accessibility/kontrast
> > I don't plan to add new features and would like after the KDEReview, to 
> > release a first version of the application, and then move it to the release 
> > service so that the application gets regularly translations improvement.
> > Thanks
> > Carl

Re: KDEReview for Kontrast

[Nate Graham]

On 8/2/20 8:29 PM, Carl Schwan wrote:

Le dimanche, août 2, 2020 2:58 PM, Nate Graham  a écrit :


Hello Carl,
This looks very useful! Overall I'd say the UI is good. One thing I find
myself missing while playing around is the ability to test system
colorschemes though. That would be a really nice addition.


I'm not sure this feature would make sense in Kontrast, but maybe this
functionality should be integrated into the new color scheme editor Noah
is building. The actual logic for calculating the contrast is very small.

What do you think?


Yeah that could work!

Nate

Re: KDEReview for Kontrast

[Albert Astals Cid]

El dijous, 30 de juliol de 2020, a les 11:16:25 CEST, Carl Schwan va escriure:
> Hi,
> I would like to move Kontrast, a contrast checker application, to KDEReview 
> Kontrast can check if two colors pass the WCAG 2.0 specification and save 
> some user's favorite color combinations.
> 
> Some screenshots of the application and a design review from the VSG is 
> available here: https://invent.kde.org/accessibility/kontrast/-/issues/1
> 
> From a code point of view, the application is very simple, but I still would 
> appreciate a general code review on it (it's my first Qt app written from 
> scratch). The code is available here: 
> https://invent.kde.org/accessibility/kontrast
> 
> I don't plan to add new features and would like after the KDEReview, to 
> release a first version of the application, and then move it to the release 
> service so that the application gets regularly translations improvement.


You don't have an icon, which is not optimal [actually i see you have an icon 
in invent.k.o so the hard part of drawing it seems to be done :)]


The # of the colors is cut for me https://i.imgur.com/1GC2sEU.png


Missing i18n:
  ./src/contents/ui/MainPage.qml:28:title: "Please choose a color"


Would be great if you had the typical --help --author, etc.
See QCommandLineParser and KAboutData::setupCommandLine


Would a documentation of the ranges make sense?
i.e. something that has the ranges and the descriptions you put for each of the 
ranges in one place? Something like a "Help" page.

  

Could only test part of the app since you're requiring unreleased Kirigami 2.14 
Which probably means your
  set(KF5_MIN_VERSION "5.70.0")
should be changed to 
  set(KF5_MIN_VERSION "5.73.0")



Out of curiosity any reason you decided to go with
   auto SavedColorModel::refresh() -> void
instead of
   void SavedColorModel::refresh()
?

Cheers,
  Albert
   

> 
> Thanks
> Carl

Re: KDEReview for Kontrast

[Nicolas Fella]

Hi Carl,

a couple of nitpicks, otherwise looks neat.

- your CMakeLists.txt does not specify a minimum Qt/KF5 version. Also
ECM 0.0.8 is likely quite old and a bit optimistic

- Setting CMAKE_CXX_STANDARD to 11 is implicitly done by ECM, no need to
do that manually. It also contradicts the
target_compile_features(kontrast PRIVATE cxx_std_17) you set later

- You can save yourself the explicit call to qt5_add_resources by adding
resources.qrc to the add_executable call. This is called AUTORCC in
cmake and ECM enables it by default

- Instead of using QScopedPointer you should be able to just
put Kontrast on the stack

- consider setting "isMenu: true" on your global drawer, that turns it
into a hamburger menu on the desktop, which is more appropriate than a
drawer


Cheers

Nico

On 30.07.20 11:16, Carl Schwan wrote:

Hi,
I would like to move Kontrast, a contrast checker application, to KDEReview. 
Kontrast can check if two colors pass the WCAG 2.0 specification and save some 
user's favorite color combinations.

Some screenshots of the application and a design review from the VSG is 
available here: https://invent.kde.org/accessibility/kontrast/-/issues/1

 From a code point of view, the application is very simple, but I still would 
appreciate a general code review on it (it's my first Qt app written from 
scratch). The code is available here: 
https://invent.kde.org/accessibility/kontrast

I don't plan to add new features and would like after the KDEReview, to release 
a first version of the application, and then move it to the release service so 
that the application gets regularly translations improvement.

Thanks
Carl

Re: KDEReview for Kontrast

[Nate Graham]

Hello Carl,
This looks very useful! Overall I'd say the UI is good. One thing I find 
myself missing while playing around is the ability to test system 
colorschemes though. That would be a really nice addition.


Nate


On 7/30/20 3:16 AM, Carl Schwan wrote:

Hi,
I would like to move Kontrast, a contrast checker application, to KDEReview. 
Kontrast can check if two colors pass the WCAG 2.0 specification and save some 
user's favorite color combinations.

Some screenshots of the application and a design review from the VSG is 
available here: https://invent.kde.org/accessibility/kontrast/-/issues/1

 From a code point of view, the application is very simple, but I still would 
appreciate a general code review on it (it's my first Qt app written from 
scratch). The code is available here: 
https://invent.kde.org/accessibility/kontrast

I don't plan to add new features and would like after the KDEReview, to release 
a first version of the application, and then move it to the release service so 
that the application gets regularly translations improvement.

Thanks
Carl

Re: kdereview - qtcurve

[David Edmundson]

​> For the record, I also asked Yichao to have a look at a comment one of
our

This issue has been resolved.

At which point, I think we're good to move? Any final objections?

Re: kdereview

[Jonathan Riddell]

On 28 September 2016 at 10:17, Ben Cooksley  wrote:
> On Wed, Sep 28, 2016 at 8:20 AM, Burkhard Lück  wrote:
>> Am Mittwoch, 21. September 2016, 11:01:18 CEST schrieb Allen Winter:
>>> I suggest we remove bodega-client, kdev-perforce, kdots, kor and kpeg from
>>> kde_projects.xml and virtually move them into scratch.
>>
>> Any objections to move bodega-client, kdev-perforce, kdots, kor and kpeg to
>> unmaintained?
>
> If nobody speaks up for them in the next 24-48 hours they can probably
> move to unmaintained I think.

Ping to maintainers or kdots and kpeg, last change before they get
moved to unmaintained.

I've just moved bodega-client and kor to unmaintained.

Jonathan

Re: kdereview - qtcurve

[David Edmundson]

On Wed, Jun 21, 2017 at 5:06 PM, Jonathan Riddell  wrote:

> On 16 June 2017 at 16:07, Yichao Yu  wrote:
> > QtCurve is now in kdereview with the intention of being in extragear/base
>
> You should set the stable branch to 1.9 in repo-metadata and ask
> translator admins to make a stable branch for translations
>

For the record, I did ask on #kde-i18n

[Wednesday, 21 June 2017] [21:38:30 BST] can I set a stable
branch of a repo in kdereview?
[Wednesday, 21 June 2017] [21:41:19 BST] please no
[Wednesday, 21 June 2017] [21:41:25 BST] gives us extra work
for no reason
[Wednesday, 21 June 2017] [21:41:29 BST] just do it once it
has moved

David

Re: kdereview - qtcurve

[David Edmundson]

-- Forwarded message --
From: "David Edmundson" <da...@davidedmundson.co.uk>
Date: 4 Jul 2017 08:23
Subject: Re: kdereview - qtcurve
To: "Albert Astals Cid" <aa...@kde.org>
Cc:



> Not fixed for me https://paste.kde.org/pzgvpred2
>
>
Thanks, that was something else.
I've fixed that.

David

Re: kdereview - qtcurve

[Albert Astals Cid]

El dijous, 22 de juny de 2017, a les 16:07:01 CEST, David Edmundson va 
escriure:
> On Mon, Jun 19, 2017 at 11:42 PM, Albert Astals Cid  wrote:
> > El divendres, 16 de juny de 2017, a les 11:07:19 CEST, Yichao Yu va
> > 
> > escriure:
> > > QtCurve is now in kdereview with the intention of being in
> > > extragear/base
> > 
> > By default it assumes i want a Qt4 build and then fails because i don't
> > have
> > the necessary packages. Maybe it could be a bit gentler and just give me a
> > nice warning?
> 
> Possibly fixed. You'll need to wipe your CMakeCache though.

Not fixed for me https://paste.kde.org/pzgvpred2

Maybe the detection of KDE4 also needs to be optional and not REQUIRED?

Cheers,
  Albert

> 
> David

Re: kdereview - qtcurve

[Albert Astals Cid]

El divendres, 16 de juny de 2017, a les 11:07:19 CEST, Yichao Yu va escriure:
> QtCurve is now in kdereview with the intention of being in extragear/base

For the record, I also asked Yichao to have a look at a comment one of our 
translators did in the i18n mailing list (via a CC'ing), haven't heard back 
about it yet, so i'm mentioning it here too.

Cheers,
  Albert

Re: kdereview - qtcurve

[David Edmundson]

On Mon, Jun 19, 2017 at 11:42 PM, Albert Astals Cid  wrote:

> El divendres, 16 de juny de 2017, a les 11:07:19 CEST, Yichao Yu va
> escriure:
> > QtCurve is now in kdereview with the intention of being in extragear/base
>
> By default it assumes i want a Qt4 build and then fails because i don't
> have
> the necessary packages. Maybe it could be a bit gentler and just give me a
> nice warning?
>
>
Possibly fixed. You'll need to wipe your CMakeCache though.

David

Re: kdereview - qtcurve

[Jonathan Riddell]

On 16 June 2017 at 16:07, Yichao Yu  wrote:
> QtCurve is now in kdereview with the intention of being in extragear/base

You should set the stable branch to 1.9 in repo-metadata and ask
translator admins to make a stable branch for translations

Jonathan

Re: kdereview - qtcurve

[David Edmundson]

> What do you need kdelibs4support for in the qt5 code?

Nothing that couldn't be ported long term, but it does currently use
KMimeData, KStandardDirs and KFileDialog.

Re: kdereview - qtcurve

[Albert Astals Cid]

El divendres, 16 de juny de 2017, a les 11:07:19 CEST, Yichao Yu va escriure:
> QtCurve is now in kdereview with the intention of being in extragear/base

By default it assumes i want a Qt4 build and then fails because i don't have 
the necessary packages. Maybe it could be a bit gentler and just give me a 
nice warning?

See https://community.kde.org/Guidelines_and_HOWTOs/CMake/
FAQs#How_can_I_make_my_package_packager-friendly.3F


What do you need kdelibs4support for in the qt5 code? 

Cheers,
  Albert

Re: kdereview: qqc2-desktop-style

[Marco Martin]

On Thu, May 18, 2017 at 11:08 AM, Marco Martin  wrote:
> Hi all,
> I'm anouncing the QtQuickControls2 desktop style in kdereview, repo name is
> qqc2-desktop-style
>
> It is intended to be a small style written in QML for QtQuickControls2 that is
> intended to be used by default in QQC2-based apps when used in the Plasma
> desktop (it shouldn't have any user-visible message, anywhere), its final
> intended location is kde/workspace to be released together with Plasma 5.11

as there don't seem to be complaints, the project should move now into workspace

--
Marco Martin

Re: kdereview: ksmtp

[Daniel Vrátil]

Hi Rolf,

thanks for the review, sorry it took me so long to get to you.

On Tuesday, May 16, 2017 8:05:17 PM CEST Rolf Eike Beer wrote:
> Am Donnerstag, 11. Mai 2017, 17:03:01 schrieb Daniel Vrátil:
> > Hi,
> > 
> > please review ksmtp, which is now in kdereview.
> 
> -the CMakeLists.txt has a mix of spaces inside () or not

Fixed

> 
> -in loginjob, line 173, you check for code 25. Should this be 250? Or is
> that 25*? Where is ServerResponse actually defined, I only see the header.

ServerResponse is defined in sessionthread.cpp for some reason. 
ServerResponse::isCode() indeed checks the prefix of the response, so 
isCode(25) returns true for any 25x return code.

> -does that support pipelining? I don't see any sync points, so I guess not.

Not yet, but it's next on the todo list.

> -there is a longstanding bug in KMail that it violates the RfC when it has a
> problem with authentication (e.g. password rejected), that is does not
> properly QUIT the SMTP session, but just closes the socket. Is that
> properly handled?

It is properly handled now. Calling Session::quit() does not close the 
connection but sends QUIT command and only closes the connection after a 
response arrives from the server (unless the server closes it first of course). 
It's now up to the user to ensure that the Session object is not destroyed 
until the state changes to Disconnected after calling Session::quit().

Dan

> 
> Greetings,
> 
> Eike


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683

signature.asc
Description: This is a digitally signed message part.

Re: kdereview: qqc2-desktop-style

[Marco Martin]

On Sun, May 21, 2017 at 7:16 PM, Albert Astals Cid  wrote:
> if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_BINARY_DIR}")
>message(FATAL_ERROR "climbinggrades bla bla bla.")
> endif()
>
> This is not climbinggrades ;)



thanks, fixed

--
Marco Martin

Re: kdereview: qqc2-desktop-style

[Albert Astals Cid]

El dijous, 18 de maig de 2017, a les 11:08:42 CEST, Marco Martin va escriure:
> Hi all,
> I'm anouncing the QtQuickControls2 desktop style in kdereview, repo name is
> qqc2-desktop-style
> 
> It is intended to be a small style written in QML for QtQuickControls2 that
> is intended to be used by default in QQC2-based apps when used in the
> Plasma desktop (it shouldn't have any user-visible message, anywhere), its
> final intended location is kde/workspace to be released together with
> Plasma 5.11

if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_BINARY_DIR}")
   message(FATAL_ERROR "climbinggrades bla bla bla.")
endif()

This is not climbinggrades ;)

Cheers,
  Albert

Re: kdereview: ksmtp

[Rolf Eike Beer]

Am Donnerstag, 11. Mai 2017, 17:03:01 schrieb Daniel Vrátil:
> Hi,
> 
> please review ksmtp, which is now in kdereview.

-the CMakeLists.txt has a mix of spaces inside () or not

-in loginjob, line 173, you check for code 25. Should this be 250? Or is that 
25*? Where is ServerResponse actually defined, I only see the header.

-does that support pipelining? I don't see any sync points, so I guess not.

-there is a longstanding bug in KMail that it violates the RfC when it has a 
problem with authentication (e.g. password rejected), that is does not 
properly QUIT the SMTP session, but just closes the socket. Is that properly 
handled?

Greetings,

Eike

signature.asc
Description: This is a digitally signed message part.

Re: kdereview: ksmtp

[Daniel Vrátil]

On Thursday, May 11, 2017 11:11:11 PM CEST Albert Astals Cid wrote:
> El dijous, 11 de maig de 2017, a les 17:03:01 CEST, Daniel Vrátil va 
escriure:
> > Hi,
> > 
> > please review ksmtp, which is now in kdereview.
> 
> Are tests supposed to pass?
> 
> 2: QFATAL : SmtpTest::testLoginJob(Plain auth ok) Received signal 11

Hmm, it passes here. Must be some timing issue. I'll see if I can reproduce it 
on my other system.

Dan


> 
> Cheers,
>   Albert
> 
> > KSMTP is a small simple library with a KJob-based API similar to KIMAP or
> > KDAV to send mime messages via SMTP. It was initially written in 2011 by
> > Gregory Schlomoff but since then it's been lying around in playground
> > without any interest or use. I have recently picked it up, ported to
> > Frameworks and improved the job handling and authentication to make the
> > library ussable for KDE PIM and would like to have it released as part of
> > KDE Applications 17.08.
> > 
> > KDE PIM currently uses a custom KIO Slave to send messages via SMTP, which
> > is hard to maintain and extend. Having a Job-based library like KSmtp will
> > make it much easier for us to introduce support for example for Google
> > XOAUTH2 authentication mechanism.
> > 
> > Thanks,
> > Dan


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

signature.asc
Description: This is a digitally signed message part.

Re: kdereview - xdg-desktop-portal-kde

[Jan Grulich]

Hi,

thank you so much for the review!!

On pátek 12. května 2017 13:21:00 CEST Lamarque Souza wrote:
> Hi,
> 
> My review:
> 
> . there are several code style inconsistencies, like "QDialog* parent" and
> "Ui::AppChooserDialog * m_dialog". In some places you use app_id while in
> other places you use appId.

Fixed.

> . you use 0 in same places that you should use nullptr.

Fixed.
 
> . there is no doxygen documentation at all.

Not sure there is reason for a documentation, for implementation of other 
portals there is documentation provided by Flatpak guys, otherwise this is 
something what should be running on the background and users shouldn't even 
know that something like this is running, they don't need to be familiar with 
this at all.

> . you can optimize
> 
> QString("%1:%2").arg(app_id).arg(id)
> 
> by doing this:
> 
> QString("%1:%2").arg(app_id, id)
> 
> since both app_id and id are QStrings.
 
Fixed.

> . in AppChooser::chooseApplication() you do
> 
> QString heading;
> ...
> heading = options.value(QLatin1String("heading")).toBool();
> 
> shouldn't heading be declared as bool?

It should be a string so it's a wrong casting.

> . in the same method you do:
> 
> if (appDialog->exec()) {
> results.insert(QLatin1String("choice"),
> appDialog->selectedApplication());
> appDialog->deleteLater();
> return 0;
> }
> 
> That means if the user rejects the dialog you never deletes it. That's a
> memory leak. You do the same thing in FileChooser::openFile(),
> FileChooser::saveFile() and Print::preparePrint().

Fixed.

> . in DesktopPortal::handleMessage() you use message.arguments().at(4)
> without checking if there are at least four arguments in message (a
> QDBusMessage object). That is risky, if someone does not provide all the
> required arguments your code can crash.

Given the dbus signature is given you shouldn't be able to call it with less 
parameters. If you do, you will get an error from dbus that the signature is 
wrong.

Jan

> 
> Lamarque V. Souza
> Linux User #57137 - https://www.linuxcounter.net/user/57137
> 
> On Fri, May 12, 2017 at 6:23 AM, Jan Grulich  wrote:
> > Hi,
> > 
> > it's been now three weeks and nobody either looked at the code or found
> > any
> > problem and raised objections. Can we proceed next and move this to place
> > where is the rest of Plasma repositories? Or is there a longer period for
> > which I have to wait until this can move on?
> > 
> > Regards,
> > Jan
> > 
> > On pátek 21. dubna 2017 8:10:36 CEST Jan Grulich wrote:
> > > Hi,
> > > 
> > > I would like to request review of xdg-desktop-portal-kde [1]. We would
> > 
> > like
> > 
> > > to make it part of Plasma releases, see [2].
> > > 
> > > What is xdg-desktop-portal-kde:
> > > It's a KDE implementation of Flatpak portals backend [3], currently with
> > > support of AppChooser, FileChooser, Notification and Print portals.
> > > 
> > > One mentioned issue on plasma-devel mailing list was usage of Qt's
> > 
> > private
> > 
> > > print API. This will most likely go away if I find out it's useless,
> > > otherwise I'll have to keep it as it's used to set CUPS properties which
> > > are not available to the outside world.
> > > 
> > > [1] - https://cgit.kde.org/xdg-desktop-portal-kde.git/
> > > [2] - https://mail.kde.org/pipermail/plasma-devel/2017-April/069506.html
> > > [3] -
> > > http://flatpak.org/xdg-desktop-portal/portal-docs.
> > 
> > html#idm140258860052032
> > 
> > > Regards,
> > > Jan

Re: kdereview - xdg-desktop-portal-kde

[Lamarque Souza]

Hi,

My review:

. there are several code style inconsistencies, like "QDialog* parent" and
"Ui::AppChooserDialog * m_dialog". In some places you use app_id while in
other places you use appId.

. you use 0 in same places that you should use nullptr.

. there is no doxygen documentation at all.

. you can optimize

QString("%1:%2").arg(app_id).arg(id)

by doing this:

QString("%1:%2").arg(app_id, id)

since both app_id and id are QStrings.

. in AppChooser::chooseApplication() you do

QString heading;
...
heading = options.value(QLatin1String("heading")).toBool();

shouldn't heading be declared as bool?

. in the same method you do:

if (appDialog->exec()) {
results.insert(QLatin1String("choice"), appDialog->
selectedApplication());
appDialog->deleteLater();
return 0;
}

That means if the user rejects the dialog you never deletes it. That's a
memory leak. You do the same thing in FileChooser::openFile(),
FileChooser::saveFile() and Print::preparePrint().

. in DesktopPortal::handleMessage() you use message.arguments().at(4)
without checking if there are at least four arguments in message (a
QDBusMessage object). That is risky, if someone does not provide all the
required arguments your code can crash.

Lamarque V. Souza

http://planetkde.org/pt-br

On Fri, May 12, 2017 at 6:23 AM, Jan Grulich  wrote:

> Hi,
>
> it's been now three weeks and nobody either looked at the code or found any
> problem and raised objections. Can we proceed next and move this to place
> where is the rest of Plasma repositories? Or is there a longer period for
> which I have to wait until this can move on?
>
> Regards,
> Jan
>
> On pátek 21. dubna 2017 8:10:36 CEST Jan Grulich wrote:
> > Hi,
> >
> > I would like to request review of xdg-desktop-portal-kde [1]. We would
> like
> > to make it part of Plasma releases, see [2].
> >
> > What is xdg-desktop-portal-kde:
> > It's a KDE implementation of Flatpak portals backend [3], currently with
> > support of AppChooser, FileChooser, Notification and Print portals.
> >
> > One mentioned issue on plasma-devel mailing list was usage of Qt's
> private
> > print API. This will most likely go away if I find out it's useless,
> > otherwise I'll have to keep it as it's used to set CUPS properties which
> > are not available to the outside world.
> >
> > [1] - https://cgit.kde.org/xdg-desktop-portal-kde.git/
> > [2] - https://mail.kde.org/pipermail/plasma-devel/2017-April/069506.html
> > [3] -
> > http://flatpak.org/xdg-desktop-portal/portal-docs.
> html#idm140258860052032
> >
> > Regards,
> > Jan
>
>
>

Re: kdereview - xdg-desktop-portal-kde

[Jan Grulich]

Hi,

it's been now three weeks and nobody either looked at the code or found any 
problem and raised objections. Can we proceed next and move this to place 
where is the rest of Plasma repositories? Or is there a longer period for 
which I have to wait until this can move on?

Regards,
Jan

On pátek 21. dubna 2017 8:10:36 CEST Jan Grulich wrote:
> Hi,
> 
> I would like to request review of xdg-desktop-portal-kde [1]. We would like
> to make it part of Plasma releases, see [2].
> 
> What is xdg-desktop-portal-kde:
> It's a KDE implementation of Flatpak portals backend [3], currently with
> support of AppChooser, FileChooser, Notification and Print portals.
> 
> One mentioned issue on plasma-devel mailing list was usage of Qt's private
> print API. This will most likely go away if I find out it's useless,
> otherwise I'll have to keep it as it's used to set CUPS properties which
> are not available to the outside world.
> 
> [1] - https://cgit.kde.org/xdg-desktop-portal-kde.git/
> [2] - https://mail.kde.org/pipermail/plasma-devel/2017-April/069506.html
> [3] -
> http://flatpak.org/xdg-desktop-portal/portal-docs.html#idm140258860052032
> 
> Regards,
> Jan

Re: kdereview: ksmtp

[Albert Astals Cid]

El dijous, 11 de maig de 2017, a les 17:03:01 CEST, Daniel Vrátil va escriure:
> Hi,
> 
> please review ksmtp, which is now in kdereview.

Are tests supposed to pass?

2: QFATAL : SmtpTest::testLoginJob(Plain auth ok) Received signal 11

Cheers,
  Albert

> 
> KSMTP is a small simple library with a KJob-based API similar to KIMAP or
> KDAV to send mime messages via SMTP. It was initially written in 2011 by
> Gregory Schlomoff but since then it's been lying around in playground
> without any interest or use. I have recently picked it up, ported to
> Frameworks and improved the job handling and authentication to make the
> library ussable for KDE PIM and would like to have it released as part of
> KDE Applications 17.08.
> 
> KDE PIM currently uses a custom KIO Slave to send messages via SMTP, which
> is hard to maintain and extend. Having a Job-based library like KSmtp will
> make it much easier for us to introduce support for example for Google
> XOAUTH2 authentication mechanism.
> 
> Thanks,
> Dan

Re: kdereview: ksmtp

[Daniel Vrátil]

Thanks, totally forgot to run clazy/krazy on the codebase.

On Thursday, May 11, 2017 10:25:36 PM CEST Allen Winter wrote:
> ran clazy level2 . no hits
> 
> ran krazy checks and it found:
> . Check for C++ ctors that should be declared 'explicit' [explicit]... 1
> issue found ./autotests/fakeserver.h: line#36 (1)

Fixed

> 
> . Check for normalized SIGNAL and SLOT signatures [normalize]... 4 issues
> found ./src/session.cpp: SIGNALS: line#285,286 (2)
> ./src/session.cpp: SLOTS: line#285,286 (2)

Fixed (converted to the new connect() syntax)

> . Check for strings used improperly or should be i18n. [strings]... 5 issues
> found ./autotests/fakeserver.cpp: QLatin1String issues
> line#172,175,190,201,218 (5)

Those are false positives. I guess krazy assumes .startsWith() to be 
QString::startsWith(), while those are QByteArray::startsWith().

Dan

> On Thursday, May 11, 2017 11:03:01 AM EDT Daniel Vrátil wrote:
> > Hi,
> > 
> > please review ksmtp, which is now in kdereview.
> > 
> > KSMTP is a small simple library with a KJob-based API similar to KIMAP or
> > KDAV to send mime messages via SMTP. It was initially written in 2011 by
> > Gregory Schlomoff but since then it's been lying around in playground
> > without any interest or use. I have recently picked it up, ported to
> > Frameworks and improved the job handling and authentication to make the
> > library ussable for KDE PIM and would like to have it released as part of
> > KDE Applications 17.08.
> > 
> > KDE PIM currently uses a custom KIO Slave to send messages via SMTP, which
> > is hard to maintain and extend. Having a Job-based library like KSmtp
> > will make it much easier for us to introduce support for example for
> > Google XOAUTH2 authentication mechanism.
> > 
> > Thanks,
> > Dan


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683

signature.asc
Description: This is a digitally signed message part.

Re: kdereview - xdg-desktop-portal-kde

[Elvis Angelaccio]

On mercoledì 3 maggio 2017 13:11:26 CEST Jan Grulich wrote:
> On středa 3. května 2017 11:28:45 CEST Elvis Angelaccio wrote:
> > On mercoledì 3 maggio 2017 07:00:14 CEST Jan Grulich wrote:
> > > > > > > Do you have xdg-desktop-portal installed?
> > > > > > 
> > > > > > Yes.
> > > > > 
> > > > > Make sure that xdg-desktop-portal is running, it should be started
> > > > > automatically by flatpak, but you never know.
> > > > 
> > > > How do I check it? Is it a process that I should see?
> > > > 
> > > > > Also try to run xdg-desktop-
> > > > > portal-kde with debug enabled using:
> > > > > QT_LOGGING_RULES=xdg-desktop-portal-kde*.debug=true
> > > > > /path/to/xdg-desktop-
> > > > > portal-kde.
> > > > 
> > > > This just prints "Desktop portal registered successfuly", which I
> > > > guess
> > > > is
> > > > ok.
> > > > 
> > > > As soon as the test app freezes (instead of opening the file dialog),
> > > > I
> > > > get
> > > > this in my journalctl: https://paste.kde.org/phcnn4uxn
> > > 
> > > I guess you have to make sure that xdg-desktop-portal-kde is installed
> > > to
> > > correct location. I have it in:
> > > -
> > > /usr/share/dbus-1/services/org.freedesktop.impl.portal.desktop.kde.servi
> > > c
> > > e - /usr/share/xdg- desktop-portal/portals/kde.portal -
> > > /usr/libexec/xdg-desktop-portal-kde
> > > 
> > > 
> > > I guess based on the log that the dbus service is important to have in
> > > correct place.
> > 
> > Thanks, this helped a bit. I had a relative path in the Exec entry of the
> > .service file. Using an absolute path fixes the file dialogs, though I
> > still don't get native notifications.
> 
> Any debug output from xdg-desktop-portal-kde or xdg-desktop-portal?

Nevermind, I reinstalled my kde runtime and now notifications work fine...

Speaking of notifications, in notification.cpp I noticed:

// TODO KNotification has no option for default action

but that's no longer true as of KNotifications 5.31: https://cgit.kde.org/
knotifications.git/commit/?id=8b161b971fae41e60a2d8093843c3fbae2429c07

Other than that, looks good to me :)

Cheers,
Elvis

> 
> > Btw it seems archlinux is using /usr/lib rather than /usr/libexec (the
> > gnome portal installs /usr/lib/xdg-desktop-portal-gtk).
> > 
> > > > > > >> Another thing, shouldn't we renamed it to
> > > > > > >> xdg-desktop-portal-plasma?
> > > > > > >> (at least the repository/package, which is what the end user is
> > > > > > >> going
> > > > > > >> to install).
> > > > > > > 
> > > > > > > Not sure, gnome folks use xdg-desktop-portal-gtk because it's
> > > > > > > only
> > > > > > > gtk
> > > > > > > related and not tied to Gnome, but we use both Qt and KDE
> > > > > > > Frameworks
> > > > > > > so
> > > > > > > I
> > > > > > > decided to go for xdg-desktop-portal-kde, there is nothing
> > > > > > > really
> > > > > > > Plasma
> > > > > > > specific.
> > > > > > 
> > > > > > The point is to integrate flatpak Qt applications with Plasma, no?
> > > > > > (plasma file picker, plasma notifications, etc.)
> > > > > > Or I can use this portal also to integrate a Qt application with,
> > > > > > say,
> > > > > > LXQt?>

Re: kdereview - xdg-desktop-portal-kde

[Elvis Angelaccio]

On mercoledì 3 maggio 2017 07:00:14 CEST Jan Grulich wrote:

> > > > > 
> > > > > Do you have xdg-desktop-portal installed?
> > > > 
> > > > Yes.
> > > 
> > > Make sure that xdg-desktop-portal is running, it should be started
> > > automatically by flatpak, but you never know.
> > 
> > How do I check it? Is it a process that I should see?
> > 
> > > Also try to run xdg-desktop-
> > > portal-kde with debug enabled using:
> > > QT_LOGGING_RULES=xdg-desktop-portal-kde*.debug=true
> > > /path/to/xdg-desktop-
> > > portal-kde.
> > 
> > This just prints "Desktop portal registered successfuly", which I guess is
> > ok.
> > 
> > As soon as the test app freezes (instead of opening the file dialog), I
> > get
> > this in my journalctl: https://paste.kde.org/phcnn4uxn
> 
> I guess you have to make sure that xdg-desktop-portal-kde is installed to
> correct location. I have it in:
> - /usr/share/dbus-1/services/org.freedesktop.impl.portal.desktop.kde.service
> - /usr/share/xdg- desktop-portal/portals/kde.portal -
> /usr/libexec/xdg-desktop-portal-kde
> 
> 
> I guess based on the log that the dbus service is important to have in
> correct place.

Thanks, this helped a bit. I had a relative path in the Exec entry of the 
.service file. Using an absolute path fixes the file dialogs, though I still 
don't get native notifications.

Btw it seems archlinux is using /usr/lib rather than /usr/libexec (the gnome 
portal installs /usr/lib/xdg-desktop-portal-gtk).


> > > > >> Another thing, shouldn't we renamed it to
> > > > >> xdg-desktop-portal-plasma?
> > > > >> (at least the repository/package, which is what the end user is
> > > > >> going
> > > > >> to install).
> > > > > 
> > > > > Not sure, gnome folks use xdg-desktop-portal-gtk because it's only
> > > > > gtk
> > > > > related and not tied to Gnome, but we use both Qt and KDE Frameworks
> > > > > so
> > > > > I
> > > > > decided to go for xdg-desktop-portal-kde, there is nothing really
> > > > > Plasma
> > > > > specific.
> > > > 
> > > > The point is to integrate flatpak Qt applications with Plasma, no?
> > > > (plasma file picker, plasma notifications, etc.)
> > > > Or I can use this portal also to integrate a Qt application with, say,
> > > > LXQt?>
> > > > 
> > > > > Jan
> > > > 
> > > > Elvis

Re: kdereview - xdg-desktop-portal-kde

[Elvis Angelaccio]

On martedì 2 maggio 2017 17:26:41 CEST Jan Grulich wrote:
> On úterý 2. května 2017 15:22:23 CEST Elvis Angelaccio wrote:
> > On Tue, May 2, 2017 at 3:09 PM, Jan Grulich  wrote:
> > > On úterý 2. května 2017 14:19:04 CEST Elvis Angelaccio wrote:
> > >> On Tue, May 2, 2017 at 12:36 PM, Jan Grulich  
wrote:
> > >> > On úterý 2. května 2017 12:21:31 CEST Aleix Pol wrote:
> > >> >> On Tue, May 2, 2017 at 12:15 PM, Albert Astals Cid 
> 
> wrote:
> > >> >> > El dimarts, 2 de maig de 2017, a les 7:22:04 CEST, Jan Grulich va
> > >> > 
> > >> > escriure:
> > >> >> >> On pondělí 1. května 2017 22:59:44 CEST Albert Astals Cid wrote:
> > >> >> >> > El divendres, 21 d’abril de 2017, a les 8:10:36 CEST, Jan
> > >> >> >> > Grulich
> > >> >> >> > va
> > >> >> >> 
> > >> >> >> escriure:
> > >> >> >> > > Hi,
> > >> >> >> > > 
> > >> >> >> > > I would like to request review of xdg-desktop-portal-kde [1].
> > >> >> >> > > We
> > >> >> >> > > would
> > >> >> >> > > like
> > >> >> >> > > to make it part of Plasma releases, see [2].
> > >> >> >> > > 
> > >> >> >> > > What is xdg-desktop-portal-kde:
> > >> >> >> > > It's a KDE implementation of Flatpak portals backend [3],
> > >> >> >> > > currently
> > >> >> >> > > with
> > >> >> >> > > support of AppChooser, FileChooser, Notification and Print
> > >> >> >> > > portals.
> > >> >> >> > > 
> > >> >> >> > > One mentioned issue on plasma-devel mailing list was usage of
> > >> >> >> > > Qt's
> > >> >> >> > > private
> > >> >> >> > > print API. This will most likely go away if I find out it's
> > >> >> >> > > useless,
> > >> >> >> > > otherwise I'll have to keep it as it's used to set CUPS
> > >> >> >> > > properties
> > >> >> >> > > which
> > >> >> >> > > are not available to the outside world.
> > >> 
> > >> Hi, doesn't seem to work here. If I click Open in your test app I get:
> > >> 
> > >> qt.qpa.qflatpakplatform.FileDialog: File dialog: selectedNameFilter()
> > >> qt.qpa.qflatpakplatform.FileDialog: File dialog: selectedFiles()
> > >> qt.qpa.qflatpakplatform.FileDialog: File dialog: show()
> > >> qt.qpa.qflatpakplatform.FileDialog: File dialog: initializeDialog()
> > >> qt.qpa.qflatpakplatform.FileDialog: Initial values:
> > >> qt.qpa.qflatpakplatform.FileDialog:Multiple files:  true
> > >> qt.qpa.qflatpakplatform.FileDialog:  Accept label:  "Open
> > >> (portal)"
> > >> qt.qpa.qflatpakplatform.FileDialog:   Window title:  "Flatpak
> > >> test - open dialog"
> > >> qt.qpa.qflatpakplatform.FileDialog: Save/Open:  Open
> > >> qt.qpa.qflatpakplatform.FileDialog:  Name filters:  ("plain
> > >> text document (*.txt *.asc *,v *.doc)", "PNG image (*.png)")
> > >> qt.qpa.qflatpakplatform.FileDialog: MimeTypes filters:
> > >> ("text/plain", "image/png")
> > >> qt.qpa.qflatpakplatform.FileDialog: Initial directory:
> > >> "file:///home/elvis" qt.qpa.qflatpakplatform.FileDialog: File dialog:
> > >> exec()
> > >> qt.qpa.qflatpakplatform.FileDialog: Couldn't get reply
> > >> qt.qpa.qflatpakplatform.FileDialog: Error:  "Did not receive a reply.
> > >> Possible causes include: the remote application did not send a reply,
> > >> the message bus security policy blocked the reply, the reply timeout
> > >> expired, or the network connection was broken."
> > >> 
> > >> but I don't see any file dialog (the app just freezes). Not sure if
> > >> related, but I also have xdg-desktop-portal-gtk installed.
> > > 
> > > Do you have xdg-desktop-portal installed?
> > 
> > Yes.
> 
> Make sure that xdg-desktop-portal is running, it should be started
> automatically by flatpak, but you never know. 

How do I check it? Is it a process that I should see?

> Also try to run xdg-desktop-
> portal-kde with debug enabled using:
> QT_LOGGING_RULES=xdg-desktop-portal-kde*.debug=true /path/to/xdg-desktop-
> portal-kde.

This just prints "Desktop portal registered successfuly", which I guess is ok.

As soon as the test app freezes (instead of opening the file dialog), I get 
this in my journalctl: https://paste.kde.org/phcnn4uxn

> 
> > >> Another thing, shouldn't we renamed it to xdg-desktop-portal-plasma?
> > >> (at least the repository/package, which is what the end user is going
> > >> to install).
> > > 
> > > Not sure, gnome folks use xdg-desktop-portal-gtk because it's only gtk
> > > related and not tied to Gnome, but we use both Qt and KDE Frameworks so
> > > I
> > > decided to go for xdg-desktop-portal-kde, there is nothing really Plasma
> > > specific.
> > 
> > The point is to integrate flatpak Qt applications with Plasma, no?
> > (plasma file picker, plasma notifications, etc.)
> > Or I can use this portal also to integrate a Qt application with, say,
> > LXQt?> 
> > > Jan
> > 
> > Elvis

Re: kdereview - xdg-desktop-portal-kde

[Jan Grulich]

On středa 3. května 2017 11:28:45 CEST Elvis Angelaccio wrote:
> On mercoledì 3 maggio 2017 07:00:14 CEST Jan Grulich wrote:
> > > > > > Do you have xdg-desktop-portal installed?
> > > > > 
> > > > > Yes.
> > > > 
> > > > Make sure that xdg-desktop-portal is running, it should be started
> > > > automatically by flatpak, but you never know.
> > > 
> > > How do I check it? Is it a process that I should see?
> > > 
> > > > Also try to run xdg-desktop-
> > > > portal-kde with debug enabled using:
> > > > QT_LOGGING_RULES=xdg-desktop-portal-kde*.debug=true
> > > > /path/to/xdg-desktop-
> > > > portal-kde.
> > > 
> > > This just prints "Desktop portal registered successfuly", which I guess
> > > is
> > > ok.
> > > 
> > > As soon as the test app freezes (instead of opening the file dialog), I
> > > get
> > > this in my journalctl: https://paste.kde.org/phcnn4uxn
> > 
> > I guess you have to make sure that xdg-desktop-portal-kde is installed to
> > correct location. I have it in:
> > -
> > /usr/share/dbus-1/services/org.freedesktop.impl.portal.desktop.kde.servic
> > e - /usr/share/xdg- desktop-portal/portals/kde.portal -
> > /usr/libexec/xdg-desktop-portal-kde
> > 
> > 
> > I guess based on the log that the dbus service is important to have in
> > correct place.
> 
> Thanks, this helped a bit. I had a relative path in the Exec entry of the
> .service file. Using an absolute path fixes the file dialogs, though I still
> don't get native notifications.

Any debug output from xdg-desktop-portal-kde or xdg-desktop-portal?

> Btw it seems archlinux is using /usr/lib rather than /usr/libexec (the gnome
> portal installs /usr/lib/xdg-desktop-portal-gtk).
> 
> > > > > >> Another thing, shouldn't we renamed it to
> > > > > >> xdg-desktop-portal-plasma?
> > > > > >> (at least the repository/package, which is what the end user is
> > > > > >> going
> > > > > >> to install).
> > > > > > 
> > > > > > Not sure, gnome folks use xdg-desktop-portal-gtk because it's only
> > > > > > gtk
> > > > > > related and not tied to Gnome, but we use both Qt and KDE
> > > > > > Frameworks
> > > > > > so
> > > > > > I
> > > > > > decided to go for xdg-desktop-portal-kde, there is nothing really
> > > > > > Plasma
> > > > > > specific.
> > > > > 
> > > > > The point is to integrate flatpak Qt applications with Plasma, no?
> > > > > (plasma file picker, plasma notifications, etc.)
> > > > > Or I can use this portal also to integrate a Qt application with,
> > > > > say,
> > > > > LXQt?>
> > > > > 

Re: kdereview - xdg-desktop-portal-kde

[Jan Grulich]

On úterý 2. května 2017 23:58:20 CEST you wrote:
> On martedì 2 maggio 2017 17:26:41 CEST Jan Grulich wrote:
> > On úterý 2. května 2017 15:22:23 CEST Elvis Angelaccio wrote:
> > > On Tue, May 2, 2017 at 3:09 PM, Jan Grulich  wrote:
> > > > On úterý 2. května 2017 14:19:04 CEST Elvis Angelaccio wrote:
> > > >> On Tue, May 2, 2017 at 12:36 PM, Jan Grulich 
> 
> wrote:
> > > >> > On úterý 2. května 2017 12:21:31 CEST Aleix Pol wrote:
> > > >> >> On Tue, May 2, 2017 at 12:15 PM, Albert Astals Cid 
> > 
> > wrote:
> > > >> >> > El dimarts, 2 de maig de 2017, a les 7:22:04 CEST, Jan Grulich
> > > >> >> > va
> > > >> > 
> > > >> > escriure:
> > > >> >> >> On pondělí 1. května 2017 22:59:44 CEST Albert Astals Cid wrote:
> > > >> >> >> > El divendres, 21 d’abril de 2017, a les 8:10:36 CEST, Jan
> > > >> >> >> > Grulich
> > > >> >> >> > va
> > > >> >> >> 
> > > >> >> >> escriure:
> > > >> >> >> > > Hi,
> > > >> >> >> > > 
> > > >> >> >> > > I would like to request review of xdg-desktop-portal-kde
> > > >> >> >> > > [1].
> > > >> >> >> > > We
> > > >> >> >> > > would
> > > >> >> >> > > like
> > > >> >> >> > > to make it part of Plasma releases, see [2].
> > > >> >> >> > > 
> > > >> >> >> > > What is xdg-desktop-portal-kde:
> > > >> >> >> > > It's a KDE implementation of Flatpak portals backend [3],
> > > >> >> >> > > currently
> > > >> >> >> > > with
> > > >> >> >> > > support of AppChooser, FileChooser, Notification and Print
> > > >> >> >> > > portals.
> > > >> >> >> > > 
> > > >> >> >> > > One mentioned issue on plasma-devel mailing list was usage
> > > >> >> >> > > of
> > > >> >> >> > > Qt's
> > > >> >> >> > > private
> > > >> >> >> > > print API. This will most likely go away if I find out it's
> > > >> >> >> > > useless,
> > > >> >> >> > > otherwise I'll have to keep it as it's used to set CUPS
> > > >> >> >> > > properties
> > > >> >> >> > > which
> > > >> >> >> > > are not available to the outside world.
> > > >> 
> > > >> Hi, doesn't seem to work here. If I click Open in your test app I
> > > >> get:
> > > >> 
> > > >> qt.qpa.qflatpakplatform.FileDialog: File dialog: selectedNameFilter()
> > > >> qt.qpa.qflatpakplatform.FileDialog: File dialog: selectedFiles()
> > > >> qt.qpa.qflatpakplatform.FileDialog: File dialog: show()
> > > >> qt.qpa.qflatpakplatform.FileDialog: File dialog: initializeDialog()
> > > >> qt.qpa.qflatpakplatform.FileDialog: Initial values:
> > > >> qt.qpa.qflatpakplatform.FileDialog:Multiple files:  true
> > > >> qt.qpa.qflatpakplatform.FileDialog:  Accept label:  "Open
> > > >> (portal)"
> > > >> qt.qpa.qflatpakplatform.FileDialog:   Window title:  "Flatpak
> > > >> test - open dialog"
> > > >> qt.qpa.qflatpakplatform.FileDialog: Save/Open:  Open
> > > >> qt.qpa.qflatpakplatform.FileDialog:  Name filters:  ("plain
> > > >> text document (*.txt *.asc *,v *.doc)", "PNG image (*.png)")
> > > >> qt.qpa.qflatpakplatform.FileDialog: MimeTypes filters:
> > > >> ("text/plain", "image/png")
> > > >> qt.qpa.qflatpakplatform.FileDialog: Initial directory:
> > > >> "file:///home/elvis" qt.qpa.qflatpakplatform.FileDialog: File dialog:
> > > >> exec()
> > > >> qt.qpa.qflatpakplatform.FileDialog: Couldn't get reply
> > > >> qt.qpa.qflatpakplatform.FileDialog: Error:  "Did not receive a reply.
> > > >> Possible causes include: the remote application did not send a reply,
> > > >> the message bus security policy blocked the reply, the reply timeout
> > > >> expired, or the network connection was broken."
> > > >> 
> > > >> but I don't see any file dialog (the app just freezes). Not sure if
> > > >> related, but I also have xdg-desktop-portal-gtk installed.
> > > > 
> > > > Do you have xdg-desktop-portal installed?
> > > 
> > > Yes.
> > 
> > Make sure that xdg-desktop-portal is running, it should be started
> > automatically by flatpak, but you never know.
> 
> How do I check it? Is it a process that I should see?
> 
> > Also try to run xdg-desktop-
> > portal-kde with debug enabled using:
> > QT_LOGGING_RULES=xdg-desktop-portal-kde*.debug=true /path/to/xdg-desktop-
> > portal-kde.
> 
> This just prints "Desktop portal registered successfuly", which I guess is
> ok.
> 
> As soon as the test app freezes (instead of opening the file dialog), I get
> this in my journalctl: https://paste.kde.org/phcnn4uxn
> 

I guess you have to make sure that xdg-desktop-portal-kde is installed to 
correct location. I have 
it in:
- /usr/share/dbus-1/services/org.freedesktop.impl.portal.desktop.kde.service - 
/usr/share/xdg-
desktop-portal/portals/kde.portal - /usr/libexec/xdg-desktop-portal-kde


I guess based on the log that the dbus service is important to have in correct 
place.

> > > >> Another thing, shouldn't we renamed it to xdg-desktop-portal-plasma?
> > > >> (at least the repository/package, which is what the end user is going
> > > >> to install).
> > > > 
> > > > Not sure, gnome folks use 

Re: kdereview - xdg-desktop-portal-kde

[Elvis Angelaccio]

On Tue, May 2, 2017 at 3:09 PM, Jan Grulich  wrote:
> On úterý 2. května 2017 14:19:04 CEST Elvis Angelaccio wrote:
>> On Tue, May 2, 2017 at 12:36 PM, Jan Grulich  wrote:
>> > On úterý 2. května 2017 12:21:31 CEST Aleix Pol wrote:
>> >> On Tue, May 2, 2017 at 12:15 PM, Albert Astals Cid  wrote:
>> >> > El dimarts, 2 de maig de 2017, a les 7:22:04 CEST, Jan Grulich va
>> >
>> > escriure:
>> >> >> On pondělí 1. května 2017 22:59:44 CEST Albert Astals Cid wrote:
>> >> >> > El divendres, 21 d’abril de 2017, a les 8:10:36 CEST, Jan Grulich va
>> >> >>
>> >> >> escriure:
>> >> >> > > Hi,
>> >> >> > >
>> >> >> > > I would like to request review of xdg-desktop-portal-kde [1]. We
>> >> >> > > would
>> >> >> > > like
>> >> >> > > to make it part of Plasma releases, see [2].
>> >> >> > >
>> >> >> > > What is xdg-desktop-portal-kde:
>> >> >> > > It's a KDE implementation of Flatpak portals backend [3],
>> >> >> > > currently
>> >> >> > > with
>> >> >> > > support of AppChooser, FileChooser, Notification and Print
>> >> >> > > portals.
>> >> >> > >
>> >> >> > > One mentioned issue on plasma-devel mailing list was usage of Qt's
>> >> >> > > private
>> >> >> > > print API. This will most likely go away if I find out it's
>> >> >> > > useless,
>> >> >> > > otherwise I'll have to keep it as it's used to set CUPS properties
>> >> >> > > which
>> >> >> > > are not available to the outside world.
>>
>> Hi, doesn't seem to work here. If I click Open in your test app I get:
>>
>> qt.qpa.qflatpakplatform.FileDialog: File dialog: selectedNameFilter()
>> qt.qpa.qflatpakplatform.FileDialog: File dialog: selectedFiles()
>> qt.qpa.qflatpakplatform.FileDialog: File dialog: show()
>> qt.qpa.qflatpakplatform.FileDialog: File dialog: initializeDialog()
>> qt.qpa.qflatpakplatform.FileDialog: Initial values:
>> qt.qpa.qflatpakplatform.FileDialog:Multiple files:  true
>> qt.qpa.qflatpakplatform.FileDialog:  Accept label:  "Open (portal)"
>> qt.qpa.qflatpakplatform.FileDialog:   Window title:  "Flatpak
>> test - open dialog"
>> qt.qpa.qflatpakplatform.FileDialog: Save/Open:  Open
>> qt.qpa.qflatpakplatform.FileDialog:  Name filters:  ("plain
>> text document (*.txt *.asc *,v *.doc)", "PNG image (*.png)")
>> qt.qpa.qflatpakplatform.FileDialog: MimeTypes filters:
>> ("text/plain", "image/png")
>> qt.qpa.qflatpakplatform.FileDialog: Initial directory:
>> "file:///home/elvis" qt.qpa.qflatpakplatform.FileDialog: File dialog:
>> exec()
>> qt.qpa.qflatpakplatform.FileDialog: Couldn't get reply
>> qt.qpa.qflatpakplatform.FileDialog: Error:  "Did not receive a reply.
>> Possible causes include: the remote application did not send a reply,
>> the message bus security policy blocked the reply, the reply timeout
>> expired, or the network connection was broken."
>>
>> but I don't see any file dialog (the app just freezes). Not sure if
>> related, but I also have xdg-desktop-portal-gtk installed.
>
> Do you have xdg-desktop-portal installed?

Yes.

>
>> Another thing, shouldn't we renamed it to xdg-desktop-portal-plasma?
>> (at least the repository/package, which is what the end user is going
>> to install).
>
> Not sure, gnome folks use xdg-desktop-portal-gtk because it's only gtk related
> and not tied to Gnome, but we use both Qt and KDE Frameworks so I decided to
> go for xdg-desktop-portal-kde, there is nothing really Plasma specific.

The point is to integrate flatpak Qt applications with Plasma, no?
(plasma file picker, plasma notifications, etc.)
Or I can use this portal also to integrate a Qt application with, say, LXQt?

>
> Jan

Elvis

Re: kdereview - xdg-desktop-portal-kde

[Elvis Angelaccio]

On Tue, May 2, 2017 at 12:36 PM, Jan Grulich  wrote:
> On úterý 2. května 2017 12:21:31 CEST Aleix Pol wrote:
>> On Tue, May 2, 2017 at 12:15 PM, Albert Astals Cid  wrote:
>> > El dimarts, 2 de maig de 2017, a les 7:22:04 CEST, Jan Grulich va
> escriure:
>> >> On pondělí 1. května 2017 22:59:44 CEST Albert Astals Cid wrote:
>> >> > El divendres, 21 d’abril de 2017, a les 8:10:36 CEST, Jan Grulich va
>> >>
>> >> escriure:
>> >> > > Hi,
>> >> > >
>> >> > > I would like to request review of xdg-desktop-portal-kde [1]. We
>> >> > > would
>> >> > > like
>> >> > > to make it part of Plasma releases, see [2].
>> >> > >
>> >> > > What is xdg-desktop-portal-kde:
>> >> > > It's a KDE implementation of Flatpak portals backend [3], currently
>> >> > > with
>> >> > > support of AppChooser, FileChooser, Notification and Print portals.
>> >> > >
>> >> > > One mentioned issue on plasma-devel mailing list was usage of Qt's
>> >> > > private
>> >> > > print API. This will most likely go away if I find out it's useless,
>> >> > > otherwise I'll have to keep it as it's used to set CUPS properties
>> >> > > which
>> >> > > are not available to the outside world.

Hi, doesn't seem to work here. If I click Open in your test app I get:

qt.qpa.qflatpakplatform.FileDialog: File dialog: selectedNameFilter()
qt.qpa.qflatpakplatform.FileDialog: File dialog: selectedFiles()
qt.qpa.qflatpakplatform.FileDialog: File dialog: show()
qt.qpa.qflatpakplatform.FileDialog: File dialog: initializeDialog()
qt.qpa.qflatpakplatform.FileDialog: Initial values:
qt.qpa.qflatpakplatform.FileDialog:Multiple files:  true
qt.qpa.qflatpakplatform.FileDialog:  Accept label:  "Open (portal)"
qt.qpa.qflatpakplatform.FileDialog:   Window title:  "Flatpak
test - open dialog"
qt.qpa.qflatpakplatform.FileDialog: Save/Open:  Open
qt.qpa.qflatpakplatform.FileDialog:  Name filters:  ("plain
text document (*.txt *.asc *,v *.doc)", "PNG image (*.png)")
qt.qpa.qflatpakplatform.FileDialog: MimeTypes filters:
("text/plain", "image/png")
qt.qpa.qflatpakplatform.FileDialog: Initial directory:  "file:///home/elvis"
qt.qpa.qflatpakplatform.FileDialog: File dialog: exec()
qt.qpa.qflatpakplatform.FileDialog: Couldn't get reply
qt.qpa.qflatpakplatform.FileDialog: Error:  "Did not receive a reply.
Possible causes include: the remote application did not send a reply,
the message bus security policy blocked the reply, the reply timeout
expired, or the network connection was broken."

but I don't see any file dialog (the app just freezes). Not sure if
related, but I also have xdg-desktop-portal-gtk installed.

Another thing, shouldn't we renamed it to xdg-desktop-portal-plasma?
(at least the repository/package, which is what the end user is going
to install).

>
> Jan

Cheers,
Elvis

Re: kdereview - xdg-desktop-portal-kde

[Jan Grulich]

On úterý 2. května 2017 14:19:04 CEST Elvis Angelaccio wrote:
> On Tue, May 2, 2017 at 12:36 PM, Jan Grulich  wrote:
> > On úterý 2. května 2017 12:21:31 CEST Aleix Pol wrote:
> >> On Tue, May 2, 2017 at 12:15 PM, Albert Astals Cid  wrote:
> >> > El dimarts, 2 de maig de 2017, a les 7:22:04 CEST, Jan Grulich va
> > 
> > escriure:
> >> >> On pondělí 1. května 2017 22:59:44 CEST Albert Astals Cid wrote:
> >> >> > El divendres, 21 d’abril de 2017, a les 8:10:36 CEST, Jan Grulich va
> >> >> 
> >> >> escriure:
> >> >> > > Hi,
> >> >> > > 
> >> >> > > I would like to request review of xdg-desktop-portal-kde [1]. We
> >> >> > > would
> >> >> > > like
> >> >> > > to make it part of Plasma releases, see [2].
> >> >> > > 
> >> >> > > What is xdg-desktop-portal-kde:
> >> >> > > It's a KDE implementation of Flatpak portals backend [3],
> >> >> > > currently
> >> >> > > with
> >> >> > > support of AppChooser, FileChooser, Notification and Print
> >> >> > > portals.
> >> >> > > 
> >> >> > > One mentioned issue on plasma-devel mailing list was usage of Qt's
> >> >> > > private
> >> >> > > print API. This will most likely go away if I find out it's
> >> >> > > useless,
> >> >> > > otherwise I'll have to keep it as it's used to set CUPS properties
> >> >> > > which
> >> >> > > are not available to the outside world.
> 
> Hi, doesn't seem to work here. If I click Open in your test app I get:
> 
> qt.qpa.qflatpakplatform.FileDialog: File dialog: selectedNameFilter()
> qt.qpa.qflatpakplatform.FileDialog: File dialog: selectedFiles()
> qt.qpa.qflatpakplatform.FileDialog: File dialog: show()
> qt.qpa.qflatpakplatform.FileDialog: File dialog: initializeDialog()
> qt.qpa.qflatpakplatform.FileDialog: Initial values:
> qt.qpa.qflatpakplatform.FileDialog:Multiple files:  true
> qt.qpa.qflatpakplatform.FileDialog:  Accept label:  "Open (portal)"
> qt.qpa.qflatpakplatform.FileDialog:   Window title:  "Flatpak
> test - open dialog"
> qt.qpa.qflatpakplatform.FileDialog: Save/Open:  Open
> qt.qpa.qflatpakplatform.FileDialog:  Name filters:  ("plain
> text document (*.txt *.asc *,v *.doc)", "PNG image (*.png)")
> qt.qpa.qflatpakplatform.FileDialog: MimeTypes filters:
> ("text/plain", "image/png")
> qt.qpa.qflatpakplatform.FileDialog: Initial directory: 
> "file:///home/elvis" qt.qpa.qflatpakplatform.FileDialog: File dialog:
> exec()
> qt.qpa.qflatpakplatform.FileDialog: Couldn't get reply
> qt.qpa.qflatpakplatform.FileDialog: Error:  "Did not receive a reply.
> Possible causes include: the remote application did not send a reply,
> the message bus security policy blocked the reply, the reply timeout
> expired, or the network connection was broken."
> 
> but I don't see any file dialog (the app just freezes). Not sure if
> related, but I also have xdg-desktop-portal-gtk installed.

Do you have xdg-desktop-portal installed?
 
> Another thing, shouldn't we renamed it to xdg-desktop-portal-plasma?
> (at least the repository/package, which is what the end user is going
> to install).

Not sure, gnome folks use xdg-desktop-portal-gtk because it's only gtk related 
and not tied to Gnome, but we use both Qt and KDE Frameworks so I decided to 
go for xdg-desktop-portal-kde, there is nothing really Plasma specific.

Jan

Re: kdereview - xdg-desktop-portal-kde

[Jan Grulich]

On úterý 2. května 2017 12:21:31 CEST Aleix Pol wrote:
> On Tue, May 2, 2017 at 12:15 PM, Albert Astals Cid  wrote:
> > El dimarts, 2 de maig de 2017, a les 7:22:04 CEST, Jan Grulich va 
escriure:
> >> On pondělí 1. května 2017 22:59:44 CEST Albert Astals Cid wrote:
> >> > El divendres, 21 d’abril de 2017, a les 8:10:36 CEST, Jan Grulich va
> >> 
> >> escriure:
> >> > > Hi,
> >> > > 
> >> > > I would like to request review of xdg-desktop-portal-kde [1]. We
> >> > > would
> >> > > like
> >> > > to make it part of Plasma releases, see [2].
> >> > > 
> >> > > What is xdg-desktop-portal-kde:
> >> > > It's a KDE implementation of Flatpak portals backend [3], currently
> >> > > with
> >> > > support of AppChooser, FileChooser, Notification and Print portals.
> >> > > 
> >> > > One mentioned issue on plasma-devel mailing list was usage of Qt's
> >> > > private
> >> > > print API. This will most likely go away if I find out it's useless,
> >> > > otherwise I'll have to keep it as it's used to set CUPS properties
> >> > > which
> >> > > are not available to the outside world.
> >> > 
> >> > Since you have copied some code from Okular maybe you can add some
> >> > other
> >> > (C) there other than RedHat's?
> >> 
> >> Added.
> >> 
> >> > What about the unusued QVariantMap  in the print.cpp file? What
> >> > are
> >> > you supposed to return there?
> >> 
> >> Seems not to be used at this moment or the portal frontend doesn't expect
> >> something to be returned with "results". I guess it's just reserved for
> >> future usage, given how complex the print API is.
> >> 
> >> > I've no idea how to use this so can't really test it :/
> >> 
> >> You can test it with this [1]. You just go to flapak-build folder and run
> >> build.sh which will generate you a flatpak repo, you add it and install
> >> using flatpak, but you also need to have xdg-desktop-portal installed.
> > 
> > Got stuck trying to figure out what to install from that local flatpak
> > repo
> > 
> > $ flatpak remote-ls mylocalrepo
> > error: GPG verification enabled, but no summary signatures found (use gpg-
> > verify-summary=false in remote config to disable)
> > 
> > And couldn't figure out how to do that, seems like the hint is only half
> > there> 
> > :D
> 
> Hi,
> Here it's explained how to use a local repo:
> https://community.kde.org/Guidelines_and_HOWTOs/Flatpak#Compile_your_applica
> tion
> 
> The catch is --no-gpg-verify.

And I think you also have to use "--user" because system-wide repositories 
actually need to be installed with gpg-verification or at least later when you 
want to use them.

Jan

Re: kdereview - xdg-desktop-portal-kde

[Aleix Pol]

On Tue, May 2, 2017 at 12:15 PM, Albert Astals Cid  wrote:
> El dimarts, 2 de maig de 2017, a les 7:22:04 CEST, Jan Grulich va escriure:
>> On pondělí 1. května 2017 22:59:44 CEST Albert Astals Cid wrote:
>> > El divendres, 21 d’abril de 2017, a les 8:10:36 CEST, Jan Grulich va
>>
>> escriure:
>> > > Hi,
>> > >
>> > > I would like to request review of xdg-desktop-portal-kde [1]. We would
>> > > like
>> > > to make it part of Plasma releases, see [2].
>> > >
>> > > What is xdg-desktop-portal-kde:
>> > > It's a KDE implementation of Flatpak portals backend [3], currently with
>> > > support of AppChooser, FileChooser, Notification and Print portals.
>> > >
>> > > One mentioned issue on plasma-devel mailing list was usage of Qt's
>> > > private
>> > > print API. This will most likely go away if I find out it's useless,
>> > > otherwise I'll have to keep it as it's used to set CUPS properties which
>> > > are not available to the outside world.
>> >
>> > Since you have copied some code from Okular maybe you can add some other
>> > (C) there other than RedHat's?
>>
>> Added.
>>
>> > What about the unusued QVariantMap  in the print.cpp file? What
>> > are
>> > you supposed to return there?
>>
>> Seems not to be used at this moment or the portal frontend doesn't expect
>> something to be returned with "results". I guess it's just reserved for
>> future usage, given how complex the print API is.
>>
>> > I've no idea how to use this so can't really test it :/
>>
>> You can test it with this [1]. You just go to flapak-build folder and run
>> build.sh which will generate you a flatpak repo, you add it and install
>> using flatpak, but you also need to have xdg-desktop-portal installed.
>
> Got stuck trying to figure out what to install from that local flatpak repo
>
> $ flatpak remote-ls mylocalrepo
> error: GPG verification enabled, but no summary signatures found (use gpg-
> verify-summary=false in remote config to disable)
>
> And couldn't figure out how to do that, seems like the hint is only half there
> :D

Hi,
Here it's explained how to use a local repo:
https://community.kde.org/Guidelines_and_HOWTOs/Flatpak#Compile_your_application

The catch is --no-gpg-verify.

Aleix

Re: kdereview - xdg-desktop-portal-kde

[Albert Astals Cid]

El dimarts, 2 de maig de 2017, a les 7:22:04 CEST, Jan Grulich va escriure:
> On pondělí 1. května 2017 22:59:44 CEST Albert Astals Cid wrote:
> > El divendres, 21 d’abril de 2017, a les 8:10:36 CEST, Jan Grulich va
> 
> escriure:
> > > Hi,
> > > 
> > > I would like to request review of xdg-desktop-portal-kde [1]. We would
> > > like
> > > to make it part of Plasma releases, see [2].
> > > 
> > > What is xdg-desktop-portal-kde:
> > > It's a KDE implementation of Flatpak portals backend [3], currently with
> > > support of AppChooser, FileChooser, Notification and Print portals.
> > > 
> > > One mentioned issue on plasma-devel mailing list was usage of Qt's
> > > private
> > > print API. This will most likely go away if I find out it's useless,
> > > otherwise I'll have to keep it as it's used to set CUPS properties which
> > > are not available to the outside world.
> > 
> > Since you have copied some code from Okular maybe you can add some other
> > (C) there other than RedHat's?
> 
> Added.
> 
> > What about the unusued QVariantMap  in the print.cpp file? What
> > are
> > you supposed to return there?
> 
> Seems not to be used at this moment or the portal frontend doesn't expect
> something to be returned with "results". I guess it's just reserved for
> future usage, given how complex the print API is.
> 
> > I've no idea how to use this so can't really test it :/
> 
> You can test it with this [1]. You just go to flapak-build folder and run
> build.sh which will generate you a flatpak repo, you add it and install
> using flatpak, but you also need to have xdg-desktop-portal installed.

Got stuck trying to figure out what to install from that local flatpak repo

$ flatpak remote-ls mylocalrepo
error: GPG verification enabled, but no summary signatures found (use gpg-
verify-summary=false in remote config to disable)

And couldn't figure out how to do that, seems like the hint is only half there 
:D

Cheers,
  Albert

> 
> [1] - https://github.com/grulja/flatpak-portal-test-kde
> 
> Regards,
> Jan

Re: kdereview - xdg-desktop-portal-kde

[Jan Grulich]

On pondělí 1. května 2017 22:59:44 CEST Albert Astals Cid wrote:
> El divendres, 21 d’abril de 2017, a les 8:10:36 CEST, Jan Grulich va 
escriure:
> > Hi,
> > 
> > I would like to request review of xdg-desktop-portal-kde [1]. We would
> > like
> > to make it part of Plasma releases, see [2].
> > 
> > What is xdg-desktop-portal-kde:
> > It's a KDE implementation of Flatpak portals backend [3], currently with
> > support of AppChooser, FileChooser, Notification and Print portals.
> > 
> > One mentioned issue on plasma-devel mailing list was usage of Qt's private
> > print API. This will most likely go away if I find out it's useless,
> > otherwise I'll have to keep it as it's used to set CUPS properties which
> > are not available to the outside world.
> 
> Since you have copied some code from Okular maybe you can add some other (C)
> there other than RedHat's?

Added.

> What about the unusued QVariantMap  in the print.cpp file? What are
> you supposed to return there?

Seems not to be used at this moment or the portal frontend doesn't expect 
something to be returned with "results". I guess it's just reserved for future 
usage, given how complex the print API is.

> I've no idea how to use this so can't really test it :/

You can test it with this [1]. You just go to flapak-build folder and run 
build.sh which will generate you a flatpak repo, you add it and install using 
flatpak, but you also need to have xdg-desktop-portal installed.

[1] - https://github.com/grulja/flatpak-portal-test-kde

Regards,
Jan

Re: kdereview - xdg-desktop-portal-kde

[Albert Astals Cid]

El divendres, 21 d’abril de 2017, a les 8:10:36 CEST, Jan Grulich va escriure:
> Hi,
> 
> I would like to request review of xdg-desktop-portal-kde [1]. We would like
> to make it part of Plasma releases, see [2].
> 
> What is xdg-desktop-portal-kde:
> It's a KDE implementation of Flatpak portals backend [3], currently with
> support of AppChooser, FileChooser, Notification and Print portals.
> 
> One mentioned issue on plasma-devel mailing list was usage of Qt's private
> print API. This will most likely go away if I find out it's useless,
> otherwise I'll have to keep it as it's used to set CUPS properties which
> are not available to the outside world.

Since you have copied some code from Okular maybe you can add some other (C) 
there other than RedHat's?

What about the unusued QVariantMap  in the print.cpp file? What are 
you supposed to return there?

I've no idea how to use this so can't really test it :/

Cheers,
  Albert

> 
> [1] - https://cgit.kde.org/xdg-desktop-portal-kde.git/
> [2] - https://mail.kde.org/pipermail/plasma-devel/2017-April/069506.html
> [3] -
> http://flatpak.org/xdg-desktop-portal/portal-docs.html#idm140258860052032
> 
> Regards,
> Jan

Re: kdereview

[Luigi Toscano]

Ben Cooksley ha scritto:
> On Sat, Oct 1, 2016 at 10:49 AM, Albert Astals Cid  wrote:
>> El divendres, 30 de setembre de 2016, a les 21:17:56 CEST, Ben Cooksley va
>> escriure:
>>> On Fri, Sep 30, 2016 at 11:27 AM, Albert Astals Cid  wrote:
 El dimecres, 21 de setembre de 2016, a les 11:01:18 CEST, Allen Winter va

 escriure:
> kwave is now moved to kdemultimedia.

 Why did that happen?
>>>
>>> It was moved at the request of it's maintainer and Allen.
>>>
 Did i miss the emails to k-c-d about reviewing it?
>>>
>>> I've just searched my mail - you didn't miss it. When the repository
>>> was moved to KDE Review the Sysadmin ticket would have expressly
>>> requested that the review process on kde-core-devel be started by the
>>> maintainer now that we'd processed the move. It appears that didn't
>>> happen.
>>>
>>> Thoughts on what should be done in regards to KWave here?
>>
>> Following the rules strictly I'd say to move it back to kdereview until we do
>> the usual steps, i.e. email to k-c-d and the module list and get the review.
>>
>> Now, moving stuff around is a bit of a pain, so if Thomas can get those 
>> emails
>> sent *NOW* I guess we can do the discussion with the kwave repo still in the
>> kdemultimedia location and if nothing critical is found we save ourselves
>> moving things back and forth.
> 
> I've not seen the email, so i've now pushed KWave back into review.
> 

... so another round of moving translations.

I would have advocated for a slightly longer waiting time.

-- 
Luigi

Re: kdereview

[Ben Cooksley]

On Sat, Oct 1, 2016 at 10:49 AM, Albert Astals Cid  wrote:
> El divendres, 30 de setembre de 2016, a les 21:17:56 CEST, Ben Cooksley va
> escriure:
>> On Fri, Sep 30, 2016 at 11:27 AM, Albert Astals Cid  wrote:
>> > El dimecres, 21 de setembre de 2016, a les 11:01:18 CEST, Allen Winter va
>> >
>> > escriure:
>> >> kwave is now moved to kdemultimedia.
>> >
>> > Why did that happen?
>>
>> It was moved at the request of it's maintainer and Allen.
>>
>> > Did i miss the emails to k-c-d about reviewing it?
>>
>> I've just searched my mail - you didn't miss it. When the repository
>> was moved to KDE Review the Sysadmin ticket would have expressly
>> requested that the review process on kde-core-devel be started by the
>> maintainer now that we'd processed the move. It appears that didn't
>> happen.
>>
>> Thoughts on what should be done in regards to KWave here?
>
> Following the rules strictly I'd say to move it back to kdereview until we do
> the usual steps, i.e. email to k-c-d and the module list and get the review.
>
> Now, moving stuff around is a bit of a pain, so if Thomas can get those emails
> sent *NOW* I guess we can do the discussion with the kwave repo still in the
> kdemultimedia location and if nothing critical is found we save ourselves
> moving things back and forth.

I've not seen the email, so i've now pushed KWave back into review.

>
> Cheers,
>   Albert

Regards,
Ben

>
>>
>> > Cheers,
>> >
>> >   Albert
>>
>> Cheers,
>> Ben
>>
>> >> No response from anyone else.
>> >>
>> >> I suggest we remove bodega-client, kdev-perforce, kdots, kor and kpeg
>> >> from
>> >> kde_projects.xml and virtually move them into scratch.
>> >>
>> >> On Sunday, September 18, 2016 12:08:41 PM Allen Winter wrote:
>> >> > Howdy,
>> >> >
>> >> > there's some stuff that's been in kdereview for a long time.
>> >> >
>> >> > according to kde_projects.xml, the kdereview programs are:
>> >> >   bodega-client (at least since Dec 2013)
>> >> >   kdev-perforce
>> >> >   kdots (at least since Nov 2015)
>> >> >   kor (at least since Oct 2014)
>> >> >   kpeg
>> >> >   kwave
>> >> >
>> >> > some of these (eg kwave) are actively developed.
>> >> >
>> >> > Can the owners of these please move them to a final location,
>> >> > clean them out, or let me know if they are really still under review?
>> >> >
>> >> > And by "move" I mean change their virtual location according to
>> >> > kde_projects or whatever "moving" means these days.
>> >> >
>> >> > -Allen
>
>

Re: kdereview

[Albert Astals Cid]

El divendres, 30 de setembre de 2016, a les 21:17:56 CEST, Ben Cooksley va 
escriure:
> On Fri, Sep 30, 2016 at 11:27 AM, Albert Astals Cid  wrote:
> > El dimecres, 21 de setembre de 2016, a les 11:01:18 CEST, Allen Winter va
> > 
> > escriure:
> >> kwave is now moved to kdemultimedia.
> > 
> > Why did that happen?
> 
> It was moved at the request of it's maintainer and Allen.
> 
> > Did i miss the emails to k-c-d about reviewing it?
> 
> I've just searched my mail - you didn't miss it. When the repository
> was moved to KDE Review the Sysadmin ticket would have expressly
> requested that the review process on kde-core-devel be started by the
> maintainer now that we'd processed the move. It appears that didn't
> happen.
> 
> Thoughts on what should be done in regards to KWave here?

Following the rules strictly I'd say to move it back to kdereview until we do 
the usual steps, i.e. email to k-c-d and the module list and get the review.

Now, moving stuff around is a bit of a pain, so if Thomas can get those emails 
sent *NOW* I guess we can do the discussion with the kwave repo still in the 
kdemultimedia location and if nothing critical is found we save ourselves 
moving things back and forth.

Cheers,
  Albert

> 
> > Cheers,
> > 
> >   Albert
> 
> Cheers,
> Ben
> 
> >> No response from anyone else.
> >> 
> >> I suggest we remove bodega-client, kdev-perforce, kdots, kor and kpeg
> >> from
> >> kde_projects.xml and virtually move them into scratch.
> >> 
> >> On Sunday, September 18, 2016 12:08:41 PM Allen Winter wrote:
> >> > Howdy,
> >> > 
> >> > there's some stuff that's been in kdereview for a long time.
> >> > 
> >> > according to kde_projects.xml, the kdereview programs are:
> >> >   bodega-client (at least since Dec 2013)
> >> >   kdev-perforce
> >> >   kdots (at least since Nov 2015)
> >> >   kor (at least since Oct 2014)
> >> >   kpeg
> >> >   kwave
> >> > 
> >> > some of these (eg kwave) are actively developed.
> >> > 
> >> > Can the owners of these please move them to a final location,
> >> > clean them out, or let me know if they are really still under review?
> >> > 
> >> > And by "move" I mean change their virtual location according to
> >> > kde_projects or whatever "moving" means these days.
> >> > 
> >> > -Allen

Re: kdereview

[Ben Cooksley]

On Fri, Sep 30, 2016 at 11:27 AM, Albert Astals Cid  wrote:
> El dimecres, 21 de setembre de 2016, a les 11:01:18 CEST, Allen Winter va
> escriure:
>> kwave is now moved to kdemultimedia.
>
> Why did that happen?

It was moved at the request of it's maintainer and Allen.

>
> Did i miss the emails to k-c-d about reviewing it?

I've just searched my mail - you didn't miss it. When the repository
was moved to KDE Review the Sysadmin ticket would have expressly
requested that the review process on kde-core-devel be started by the
maintainer now that we'd processed the move. It appears that didn't
happen.

Thoughts on what should be done in regards to KWave here?

>
> Cheers,
>   Albert

Cheers,
Ben

>
>> No response from anyone else.
>>
>> I suggest we remove bodega-client, kdev-perforce, kdots, kor and kpeg from
>> kde_projects.xml and virtually move them into scratch.
>>
>> On Sunday, September 18, 2016 12:08:41 PM Allen Winter wrote:
>> > Howdy,
>> >
>> > there's some stuff that's been in kdereview for a long time.
>> >
>> > according to kde_projects.xml, the kdereview programs are:
>> >   bodega-client (at least since Dec 2013)
>> >   kdev-perforce
>> >   kdots (at least since Nov 2015)
>> >   kor (at least since Oct 2014)
>> >   kpeg
>> >   kwave
>> >
>> > some of these (eg kwave) are actively developed.
>> >
>> > Can the owners of these please move them to a final location,
>> > clean them out, or let me know if they are really still under review?
>> >
>> > And by "move" I mean change their virtual location according to
>> > kde_projects or whatever "moving" means these days.
>> >
>> > -Allen
>
>

Re: kdereview

[Albert Astals Cid]

El dimecres, 21 de setembre de 2016, a les 11:01:18 CEST, Allen Winter va 
escriure:
> kwave is now moved to kdemultimedia.

Why did that happen?

Did i miss the emails to k-c-d about reviewing it?

Cheers,
  Albert

> No response from anyone else.
> 
> I suggest we remove bodega-client, kdev-perforce, kdots, kor and kpeg from
> kde_projects.xml and virtually move them into scratch.
> 
> On Sunday, September 18, 2016 12:08:41 PM Allen Winter wrote:
> > Howdy,
> > 
> > there's some stuff that's been in kdereview for a long time.
> > 
> > according to kde_projects.xml, the kdereview programs are:
> >   bodega-client (at least since Dec 2013)
> >   kdev-perforce
> >   kdots (at least since Nov 2015)
> >   kor (at least since Oct 2014)
> >   kpeg
> >   kwave
> > 
> > some of these (eg kwave) are actively developed.
> > 
> > Can the owners of these please move them to a final location,
> > clean them out, or let me know if they are really still under review?
> > 
> > And by "move" I mean change their virtual location according to
> > kde_projects or whatever "moving" means these days.
> > 
> > -Allen

Re: kdereview

[Ben Cooksley]

On Wed, Sep 28, 2016 at 9:06 AM, Allen Winter  wrote:
> On Tuesday, September 27, 2016 09:20:32 PM Burkhard Lück wrote:
>> Am Mittwoch, 21. September 2016, 11:01:18 CEST schrieb Allen Winter:
>> > I suggest we remove bodega-client, kdev-perforce, kdots, kor and kpeg from
>> > kde_projects.xml and virtually move them into scratch.
>>
>> Any objections to move bodega-client, kdev-perforce, kdots, kor and kpeg to
>> unmaintained?
>>
> kdev-perforce should be moved into kdevplatform or kdevelop/plugins or 
> somesuch.
> I asked Ben to do that last week.  Not sure if that has been done yet.

I haven't moved it, as I was under the impression the repository was
defacto dead, as it's code had been merged into kdevplatform.
Not sure if the repository was going to be mothballed or deleted once
KDevPlatform 5.1 was released though.

>
> I have heard nothing about bodega-client, kdots, kor, kpeg since my initial 
> inquiry.
>

Cheers,
Ben

Re: kdereview

[Luigi Toscano]

Il 27 settembre 2016 22:06:35 CEST, Allen Winter  ha scritto:
>On Tuesday, September 27, 2016 09:20:32 PM Burkhard Lück wrote:
>> Am Mittwoch, 21. September 2016, 11:01:18 CEST schrieb Allen Winter:
>> > I suggest we remove bodega-client, kdev-perforce, kdots, kor and
>kpeg from
>> > kde_projects.xml and virtually move them into scratch.
>> 
>> Any objections to move bodega-client, kdev-perforce, kdots, kor and
>kpeg to 
>> unmaintained?
>> 
>kdev-perforce should be moved into kdevplatform or kdevelop/plugins or
>somesuch.
>I asked Ben to do that last week.  Not sure if that has been done yet.
>
>I have heard nothing about bodega-client, kdots, kor, kpeg since my
>initial inquiry.

I would leave kdev-perforce where it is until kdevelop 5.1 is released, as the 
code has been integrated there. 

Ciao

-- 
Luigi

Re: kdereview

[Allen Winter]

On Tuesday, September 27, 2016 09:20:32 PM Burkhard Lück wrote:
> Am Mittwoch, 21. September 2016, 11:01:18 CEST schrieb Allen Winter:
> > I suggest we remove bodega-client, kdev-perforce, kdots, kor and kpeg from
> > kde_projects.xml and virtually move them into scratch.
> 
> Any objections to move bodega-client, kdev-perforce, kdots, kor and kpeg to 
> unmaintained?
> 
kdev-perforce should be moved into kdevplatform or kdevelop/plugins or somesuch.
I asked Ben to do that last week.  Not sure if that has been done yet.

I have heard nothing about bodega-client, kdots, kor, kpeg since my initial 
inquiry.

Re: kdereview

[Luigi Toscano]

Morten Volden ha scritto:
> Hi Allen
> 
> Sorry for the late reply.
> 
> kdev-perforce has already been moved to into kdevplatform (So I guess that
> qualifies as it passing review.).

Does it mean that the code was merged inside the kdevplatform repository? But
then you did not add the Messages.sh file, so no translation extracted. I'm
going to fix this.


> 
> If the repo has to be moved I suggest to move it into kdevelop/plugins.

As the code was merged, the repository should be probably cleaned, as it
happened with other repositories whose code was merged in kdevplatform or in
other components (i.e. add a commit which removes all files and adds a README
which points to the new location). Then we can remove it/move to unmaintained.

Ciao
-- 
Luigi

Re: kdereview

[Luigi Toscano]

On Wednesday, 21 September 2016 11:01:18 CEST Allen Winter wrote:
> kwave is now moved to kdemultimedia.
> No response from anyone else.
> 
> I suggest we remove bodega-client, kdev-perforce, kdots, kor and kpeg from
> kde_projects.xml and virtually move them into scratch.

More than "remove", there is specific "unmaintained" bucket in repo-metadata.

-- 
Luigi

Re: kdereview

[Allen Winter]

kwave is now moved to kdemultimedia.
No response from anyone else.

I suggest we remove bodega-client, kdev-perforce, kdots, kor and kpeg from 
kde_projects.xml
and virtually move them into scratch.

On Sunday, September 18, 2016 12:08:41 PM Allen Winter wrote:
> Howdy,
> 
> there's some stuff that's been in kdereview for a long time.
> according to kde_projects.xml, the kdereview programs are:
> 
>   bodega-client (at least since Dec 2013)
>   kdev-perforce
>   kdots (at least since Nov 2015)
>   kor (at least since Oct 2014)
>   kpeg
>   kwave
> 
> some of these (eg kwave) are actively developed.
> 
> Can the owners of these please move them to a final location,
> clean them out, or let me know if they are really still under review?
> 
> And by "move" I mean change their virtual location according to kde_projects
> or whatever "moving" means these days.
> 
> -Allen
> 
> 

Re: KDEReview: Nepomuk-Controller QML

[Jörg Ehrichs]

 I guess which icons are shown in the system tray is a setting of the system
 tray applet, no?

Yes but I have now a proper plasma update script, so this issue should
be solved now.
The script will be installed by default so any user should be updated correctly

Re: KDEReview: Nepomuk-Controller QML

[Jörg Ehrichs]

 I'd say the Messages.sh pot filename are wrong for both the dataengine and the
 applet. Please confirm with the plasma developers what's the catalog name that
 will be loaded but yours doesn't seem to follow the pattern.


I see, the i18n techbase article says every qml app needs
plasma_applet_ infront of the pot filename
same for DataEngine and plasma_engine. I've changed the Messages.sh
and some related problems.

Thanks for the Hint.

Re: KDEREVIEW: StackFolder plasma applet

[Nicolas Lécureuil]

On Wed, Dec 12, 2012 at 5:30 PM, Jos Poortvliet j...@opensuse.org wrote:
 On Wednesday 07 November 2012 13:24:30 Ural Mullabaev wrote:
 Hello all!

 Let me introduce you our applet - StackFolder. It has been moved to
 kdereview to get into kdeplasma-addons. StackFolder is a plasmoid that
 provides fast and convenient access to most frequently used files and
 directories. To start working with a folder in StackFolder, drag it from
 Dolphin to the panel and in the pop-up menu choose Stack Folder.
 Initially StackFolder was based on FolderView applet. We developed new
 smooth user interface in QML. StackFolder is a PopupApplet that allows to
 browse folders and quickly access files in them as well as instantly
 preview a wide range of different file types (including pictures, video
 and audio) via tight integration with another ROSA utility - KLook. We
 thought over many small but useful features like StackFolder's icon
 animation on addition of new file in download folder or files sorting by
 addition date.

 https://projects.kde.org/projects/kdereview/stackfolder
 http://wiki.rosalab.ru/en/index.php/What_is_StackFolder
 http://www.youtube.com/watch?v=DnH13y_FlHU

 Hi Ural,

 Nice work on it, the youtube video is cool ;-)

 A question out of curiosity: do you intend to have this as QML replacement
 of Folderview?


i would prefer something like a merge, and have an option to use
folderview OR stackfolder.

--
Cordialement,
Nicolas Lécureuil
Mageia KDE Team
JID: neocl...@jabber.fr

Re: KDEReview: Nepomuk-Controller QML

[Kevin Krammer]

On Wednesday, 2013-03-27, Jörg Ehrichs wrote:
  Assuming this setting is stored in some KConfig based file, wouldn't it
  be possible to migrate it using kconf_update?
 
 Good question. The config is saved in plasma-desktop-appletsrc
 
 and I need to add:
 
 [Containments][3][Applets][25][Configuration][Applets][36]
 geometry=0,0,24,24
 immutability=1
 plugin=nepomukcontroller-qml
 zvalue=0
 
 it seems the old nepomuk controller did not save to this file
 instead if this program will be removed from the system it won't start.
 Have to explicitly start it via $ nepomukcontroller to start it here

I guess which icons are shown in the system tray is a setting of the system 
tray applet, no?

Cheers,
Kevin
-- 
Kevin Krammer, KDE developer, xdg-utils developer
KDE user support, developer mentoring


signature.asc
Description: This is a digitally signed message part.

Re: KDEReview: Nepomuk-Controller QML

[Jörg Ehrichs]

 Assuming this setting is stored in some KConfig based file, wouldn't it be
 possible to migrate it using kconf_update?


Good question. The config is saved in plasma-desktop-appletsrc

and I need to add:

[Containments][3][Applets][25][Configuration][Applets][36]
geometry=0,0,24,24
immutability=1
plugin=nepomukcontroller-qml
zvalue=0

it seems the old nepomuk controller did not save to this file
instead if this program will be removed from the system it won't start.
Have to explicitly start it via $ nepomukcontroller to start it here

Re: KDEReview: Nepomuk-Controller QML

[Jörg Ehrichs]

 The minor version upgrade process all throughout the KDE/Plasma 4 releases
 started off as a fairly steady source of minor nits and irritants for users.

 The Nepomuk controls themselves are already fairly well-known so it's even
 more important IMO that if they are deprecated that they don't simply
 disappear completely the first time a new KDE 4.11 user logs in.


That is true, I do hope though to increase the awareness of what Nepomuk does
in the background by exposing not just the fileindexer but also especially
the workload the pim indexer is doing. While the look and feel of the systray
will be completely different it is mostly an improvement of the currently rather
hidden dialog that comes up.

One detail though is missing, the old systray was able give a number of
indexer file resources. This number is missing from my QML approach.

Not sure how important this number is, as it isn't really a good measure or
even a measure at all how big the Nepomuk database is at all.


 Likewise it would be a good idea to ensure our Release Notes for 4.11 (if any
 are started) reflect the migration so that our packagers can ensure they
 change package dependencies if deemed appropriate.


I would write a blogpost about this, as soon as this goes into SC. IS there any
other action I need to do to ensure packagers are aware of the change?

The current nepomukcontroller is enabled in the systray (right click on the
systray-settings-enable additional entries) this step must be done
for the new nepomuk controller to allow a seamingless transition.


 Not trying to discourage, but just some things to think about.


Don't worry, exactly these kind of thoughts are why the review process exist.
I'd rather have overly worrying reviewers than being responsible for
an unpleasant
transition due to my change.

Kind Regards,
Joerg

Re: KDEReview: Nepomuk-Controller QML

[Michael Pyne]

On Tuesday, March 26, 2013 11:42:46 Jörg Ehrichs wrote:
 One detail though is missing, the old systray was able give a number of
 indexer file resources. This number is missing from my QML approach.
 
 Not sure how important this number is, as it isn't really a good measure or
 even a measure at all how big the Nepomuk database is at all.

Well we rearrange U/I even across point releases (if it fixes bugs) so merely 
removing a useless stat is not a big concern, as long as the overall 
functionality is still comparable to what was present before. Removing the 
number might even be judged as a U/I improvement (why show something neither 
the user nor developer will need except possibly for bug triage?).

  Likewise it would be a good idea to ensure our Release Notes for 4.11 (if
  any are started) reflect the migration so that our packagers can ensure
  they change package dependencies if deemed appropriate.
 
 I would write a blogpost about this, as soon as this goes into SC. IS there
 any other action I need to do to ensure packagers are aware of the change?

The release notes is the big thing, I would expect packagers are tracking that 
Wiki page or web page as they are put together during the release series so 
that they can make preparations ahead-of-time.

The only other thing you'd want to do is to send an email to the kde-packager 
mailing list informing them of the change and any action that might be needed 
or desirable on their part to ensure a smooth transition for their users. The 
mailing list is moderated so expect that; your message will make it through 
regardless for this topic.

Regards,
 - Michael Pyne

signature.asc
Description: This is a digitally signed message part.

Re: KDEReview: Nepomuk-Controller QML

[Vishesh Handa]

On Sat, Mar 23, 2013 at 8:12 PM, Jörg Ehrichs joerg.ehri...@gmx.de wrote:

 So after a first introduction in plasma, I like to get this thing into SC.

 The Nepomuk-Controller aims to replace the current system tray Nepomuk
 applet.
 This one allows to suspend/resume and show information for all the
 indexers, thus
 this gives any user a small hint what happens in the background and allow
 them
 to suspend it, if they need to.

 For the review I have pushed the branch nepomukcontroller-qml into
 kde-workspace.

 You can find the important parts at
 plasma/generic/dataengines/nepomuk
 plasma/generic/applet/nepomuk-controller

 The dataengine as well as the applet is only installed if
 Nepomuk-Core/Soprano is found
 as build dependency.

 This would deprecate:
 kde-runtime/nepomuk/controller/


Deprecate or remove? Cause I would like to remove it.



 and need current nepomuk-core master

 For an easy to install version there is also:
 http://quickgit.kde.org/?p=scratch/jehrichs/nepomukcontroller-qml.git

 Some Pictures:
 http://wstaw.org/m/2013/03/20/nepomukcontroller-qml1.jpg
 http://wstaw.org/m/2013/03/20/nepomukcontroller-qml2.jpg
 http://wstaw.org/m/2013/03/20/nepomukcontroller-qml3.jpg
 http://wstaw.org/m/2013/03/20/nepomukcontroller-qml4.jpg

 Any help/comments are welcome.
 Since my first mail on the plasma devel list, the dbus calls are
 asynchrone now, the FileWatch service is not shown on default, but can
 be enabled if the user wants, and the licence should be fine now.

 Kind Regards
 Joerg




-- 
Vishesh Handa

Re: KDEReview: Nepomuk-Controller QML

[Michael Pyne]

On Monday, March 25, 2013 16:46:10 Vishesh Handa wrote:
 On Sat, Mar 23, 2013 at 8:12 PM, Jörg Ehrichs joerg.ehri...@gmx.de wrote:
  The dataengine as well as the applet is only installed if
  Nepomuk-Core/Soprano is found
  as build dependency.
  
  This would deprecate:
  kde-runtime/nepomuk/controller/
 
 Deprecate or remove? Cause I would like to remove it.

What will the provision be to automatically replace the old systray program 
with the applet (or at least let the user know where to add the new applet 
from)?

The minor version upgrade process all throughout the KDE/Plasma 4 releases 
started off as a fairly steady source of minor nits and irritants for users. 

The Nepomuk controls themselves are already fairly well-known so it's even 
more important IMO that if they are deprecated that they don't simply 
disappear completely the first time a new KDE 4.11 user logs in.

Likewise it would be a good idea to ensure our Release Notes for 4.11 (if any 
are started) reflect the migration so that our packagers can ensure they 
change package dependencies if deemed appropriate.

Not trying to discourage, but just some things to think about.

Regards,
 - Michael Pyne

signature.asc
Description: This is a digitally signed message part.

Re: KDEReview: Nepomuk-Controller QML

[Albert Astals Cid]

El Dissabte, 23 de març de 2013, a les 15:42:33, Jörg Ehrichs va escriure:
 So after a first introduction in plasma, I like to get this thing into SC.
 
 The Nepomuk-Controller aims to replace the current system tray Nepomuk
 applet. This one allows to suspend/resume and show information for all the
 indexers, thus
 this gives any user a small hint what happens in the background and allow
 them to suspend it, if they need to.
 
 For the review I have pushed the branch nepomukcontroller-qml into
 kde-workspace.

Why workspace when you are deprecating something in runtime?

Cheers,
  Albert

Re: KDEReview: Nepomuk-Controller QML

[Jörg Ehrichs]

 Why workspace when you are deprecating something in runtime?


All other Plasma::DataEngines and applets (battery, network) are in
workspace too. While runtime/plasma has only some generic stuff.

So in general it felt like the better place.

In addition I think this service info tool is not a runtime dependency,
like the KCM, kioslaves for nepomuk and the systray seemed to
have ended in runtime only, because it got added there all together.

Regards,
Joerg

Re: KDEREVIEW: share like connect and plasmate

[Ben Cooksley]

On Thu, Jan 31, 2013 at 12:47 PM, Aaron J. Seigo ase...@kde.org wrote:
 On Thursday, January 31, 2013 10:43:29 Ben Cooksley wrote:
  as it has been mentioned plasmate is ready to go into the extragear.
  Can you move it to the extragear?

 Where precisely in Extragear is Plasmate

 SDK

 and Share-Like-Connect headed?

 Base

 thanks :)

Both Plasmate and Share-Like-Connect have now been moved to the
appropriate locations.


 --
 Aaron J. Seigo

Regards,
Ben Cooksley
KDE Sysadmin

Re: KDEREVIEW: share like connect and plasmate

[Ben Cooksley]

On Wed, Jan 30, 2013 at 11:23 PM, Giorgos Tsiapaliokas
terie...@gmail.com wrote:
 Hello,

Hi Giorgos,


 as it has been mentioned plasmate is ready to go into the extragear.
 Can you move it to the extragear?

Where precisely in Extragear is Plasmate and Share-Like-Connect headed?
They are both overdue to be moved from KDE Review.

Note: Moves into and out of KDE Review must *always* be CC'ed to kde-core-devel.

On that note: any last objections?


 Regards,
 Giorgos

Regards,
Ben Cooksley
KDE Sysadmin



 --
 Giorgos Tsiapaliokas (terietor)
 KDE Developer

 terietor.gr

 ___
 Plasma-devel mailing list
 plasma-de...@kde.org
 https://mail.kde.org/mailman/listinfo/plasma-devel

Re: KDEREVIEW: share like connect and plasmate

[Aaron J. Seigo]

On Thursday, January 31, 2013 10:43:29 Ben Cooksley wrote:
  as it has been mentioned plasmate is ready to go into the extragear.
  Can you move it to the extragear?
 
 Where precisely in Extragear is Plasmate

SDK

 and Share-Like-Connect headed?

Base

thanks :)

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.

Re: KDEREVIEW: Mangonel

[Ben Cooksley]

On Wed, Jan 23, 2013 at 7:13 PM, Martin Sandsmark
martin.sandsm...@kde.org wrote:
 Distinguished Sirs and Madams,

 On Tue, Jan 08, 2013 at 09:08:11PM +0100, Martin Sandsmark wrote:
 Mangonel has just been moved to KDE Review.

 The KDE Review process is set to be two weeks, and if Mangonel calculator
 isn't completely wrong, that period is now up.

 Since all the issues raised are fixed, I assume I can now move forward with
 moving it to extragear/base?

Mangonel has now been moved to Extragear Base.


 Thanks to everyone who have reviewed, and contributed comments and fixes.

 --
 Martin Sandsmark

Regards,
Ben

Re: KDEREVIEW: Mangonel

[Martin Sandsmark]

Distinguished Sirs and Madams,

On Tue, Jan 08, 2013 at 09:08:11PM +0100, Martin Sandsmark wrote:
 Mangonel has just been moved to KDE Review.

The KDE Review process is set to be two weeks, and if Mangonel calculator
isn't completely wrong, that period is now up.

Since all the issues raised are fixed, I assume I can now move forward with
moving it to extragear/base?

Thanks to everyone who have reviewed, and contributed comments and fixes.

-- 
Martin Sandsmark

Re: KDEREVIEW: Mangonel

[Martin Sandsmark]

On Wed, Jan 09, 2013 at 09:07:13PM +0200, Yuri Chornoivan wrote:
 If there is no Help button and no desktop file, there is not much
 sense in making docbooks.

I agree.


 I propose just add an item to UserBase launchers list [1] and some
 tiny page based on README.md.

I'll be sure to market it wherever I can when I finally release it. :-)


-- 
Martin Sandsmark

Re: Re: KDEREVIEW: Mangonel

[Bart Kroon]

Hello List,

On Tuesday 08 January 2013 23:12:01 Albert Astals Cid wrote:
 El Dimarts, 8 de gener de 2013, a les 21:08:11, Martin Sandsmark va 
escriure:
  Dear Sirs and Madams,
  
  Mangonel has just been moved to KDE Review.
  
  Mangonel is a simple and lightweight application launcher in the vein of
  Katapult from ye olde KDE 3 days, kind of reminiscent of the task
  oriented UI for krunner, but dropping the slow krunner architecture in
  favour of a simpler and more straightforward one that aims to give
  immediate feedback to the user. So it doesn't do everything krunner does,
  but should be useful for some people as either an alternative to or
  complement to krunner.
  
  It was originally developed by Bart Kroon, but he is currently too busy to
  work on it, so I got a go-ahead from him to clean it up for a proper
  release with licenses and stuff (which I think it needs because it's
  pretty useful and should be spread far and wide).
 
 Which is its intended destination extragear-something?
 
 Any reason not to use bugs.kde.org?
It was not in KDE so it did not use bugs.kde.org. This would be appropriate 
when it is adopted obviously.
 
 The i18n is quite broken, a simple grep gives me
 ./Config.cpp:39:this-setWindowTitle(KApplication::instance()-
 
 applicationName() + QString( - Configuration));
 
 ./Config.cpp:48:QLabel* hotkeyLabel = new QLabel(Shortcut to show
 Mangonel:, this);
 ./Mangonel.cpp:74:m_actionShow = new KAction(QString(Show Mangonel),
 this);
 ./Mangonel.cpp:92:QAction* actionConfig = new
 QAction(KIcon(configure), Configuration, this);
 ./Mangonel.cpp:480:m_descriptionLabel = new QGraphicsTextItem(( +
 application.type + ), this);
Translations are indeed not a thing this program currently supports. There are 
not too many strings in there so this should not pose to much of a problem.
 
 Besides all those units in units.c seem untranslatable.
 
 Any reason for not using kunitconversion in there?
I didn't know about this. This seems appropriate.
 
 Also the units.c copytight header looks a bit scary
This was realy a quick hack add on that still needs a propper implementation. 
It doesn't even work properly as it is now.
 
 Cheers,
   Albert

- Bart

Re: KDEREVIEW: Mangonel

[Jekyll Wu]

On 2013年01月09日 08:09, Martin Sandsmark wrote:

Any reason not to use bugs.kde.org?

Fixed.




Hi, I see you made the change :

-aboutData-setBugAddress(QByteArray(bugs.mango...@tarmack.eu));
+aboutData-setBugAddress(QByteArray(https://bugs.kde.org/;));

Hmm, that is not going to work. Dr.Konqi expects sub...@bugs.kde.org 
as the bug address of applications using bugs.kde.org. If you plan to 
use bugs.kde.org as the tracker, then you don't need to call 
setBugAddress() at all. The default value just works.


And don't forget to ask sysadmins to create a mangonel product on 
bugs.kde.org :)


Regards
Jekyll

Re: KDEREVIEW: Mangonel

[Martin Sandsmark]

On Wednesday 09 January 2013 08:56:25 Jekyll Wu wrote:
 If you plan to 
 use bugs.kde.org as the tracker, then you don't need to call 
 setBugAddress() at all. The default value just works.

Fixed.


 And don't forget to ask sysadmins to create a mangonel product on 
 bugs.kde.org :)

Done.

Thanks for the review! :D

-- 
Martin Sandsmark
KDE

Re: Re: KDEREVIEW: Mangonel

[Martin Gräßlin]

On Wednesday 09 January 2013 18:35:49 Martin Sandsmark wrote:
 On Tuesday 08 January 2013 19:30:45 Allen Winter wrote:
  No docbook manual
 
 I guess I'll to contact the doc team for this?
 
  Do you want apidox generated? if so you also need a Mainpage.dox
 
 No need for that (yet, at least, we might want to make it plugin-based in
 the future).
  Bart's email address is missing from the Copyright statements in many
  files.
 On purpose, I didn't want to spread his email address unencoded all over the
 internet.
There is one mail already on the list, so it is already spread.

signature.asc
Description: This is a digitally signed message part.

Re: KDEREVIEW: Mangonel

[Albert Astals Cid]

El Dimecres, 9 de gener de 2013, a les 01:09:28, Martin Sandsmark va escriure:
 Hi, thanks for the review!
 
 On Tuesday 08 January 2013 23:12:01 Albert Astals Cid wrote:
  Which is its intended destination extragear-something?
 
 Yes, sorry, I forgot to mention, it is destined for extragear/base.
 
  Any reason not to use bugs.kde.org?
 
 Fixed.
 
  The i18n is quite broken, a simple grep gives me
  ./Config.cpp:39:this-setWindowTitle(KApplication::instance()-
  
  applicationName() + QString( - Configuration));
  
  ./Config.cpp:48:QLabel* hotkeyLabel = new QLabel(Shortcut to show
  Mangonel:, this);
  ./Mangonel.cpp:74:m_actionShow = new KAction(QString(Show Mangonel),
  this);
  ./Mangonel.cpp:92:QAction* actionConfig = new
  QAction(KIcon(configure), Configuration, this);
 
 Fixed.
 
  ./Mangonel.cpp:480:m_descriptionLabel = new QGraphicsTextItem((
  +
  application.type + ), this);
 
 application.type should already be translated, is it problematic that I wrap
 it in parentheses?

You should probably make it look like
 i18nc(some context of what stuff is, (%1), application.type)

Just in case someone needs to do something different with the parenthesis.

 
  Besides all those units in units.c seem untranslatable.
  Any reason for not using kunitconversion in there?
 
 Ported it to use KUnitConversion now.
 
  Also the units.c copytight header looks a bit scary
 
 Since they're not necessary anymore I just deleted units.c/units.h.
 
 (I hope I don't need to eradicate them from the git history?)

No idea if we really need to be such purists

Cheers,
  Albert

 
 
 
 And lastly, I also forgot to thank Bart for this great app. :-)

Re: KDEREVIEW: Mangonel

[Martin Sandsmark]

On Wednesday 09 January 2013 19:49:36 Albert Astals Cid wrote:
 You should probably make it look like
  i18nc(some context of what stuff is, (%1), application.type)
 
 Just in case someone needs to do something different with the parenthesis.

Okay, fixed. Thanks :-)

-- 
Martin Sandsmark
KDE

Re: KDEREVIEW: Mangonel

[Martin Sandsmark]

On Wednesday 09 January 2013 19:45:27 Martin Gräßlin wrote:
 There is one mail already on the list, so it is already spread.

Okay, so the proverbial cat's out of the bag I guess, so I just added his
email to the license headers.

-- 
Martin Sandsmark
KDE

Re: KDEREVIEW: Mangonel

[Yuri Chornoivan]

написане Wed, 09 Jan 2013 19:35:49 +0200, Martin Sandsmark  
martin.sandsm...@kde.org:



On Tuesday 08 January 2013 19:30:45 Allen Winter wrote:

No docbook manual

I guess I'll to contact the doc team for this?


If there is no Help button and no desktop file, there is not much sense  
in making docbooks.


I propose just add an item to UserBase launchers list [1] and some tiny  
page based on README.md.


Just my two cents.

Best regards,
Yuri


[1] http://userbase.kde.org/Plasma_application_launchers

Re: KDEREVIEW: Mangonel

[Allen Winter]

On Wednesday 09 January 2013 09:07:13 PM Yuri Chornoivan wrote:
 написане Wed, 09 Jan 2013 19:35:49 +0200, Martin Sandsmark  
 martin.sandsm...@kde.org:
 
  On Tuesday 08 January 2013 19:30:45 Allen Winter wrote:
  No docbook manual
  I guess I'll to contact the doc team for this?
 
 If there is no Help button and no desktop file, there is not much sense  
 in making docbooks.
 
 I propose just add an item to UserBase launchers list [1] and some tiny  
 page based on README.md.
 
Good idea.

 
 [1] http://userbase.kde.org/Plasma_application_launchers

Re: KDEREVIEW: Mangonel

[Albert Astals Cid]

El Dimarts, 8 de gener de 2013, a les 21:08:11, Martin Sandsmark va escriure:
 Dear Sirs and Madams,
 
 Mangonel has just been moved to KDE Review.
 
 Mangonel is a simple and lightweight application launcher in the vein of
 Katapult from ye olde KDE 3 days, kind of reminiscent of the task oriented
 UI for krunner, but dropping the slow krunner architecture in favour of a
 simpler and more straightforward one that aims to give immediate feedback
 to the user. So it doesn't do everything krunner does, but should be useful
 for some people as either an alternative to or complement to krunner.
 
 It was originally developed by Bart Kroon, but he is currently too busy to
 work on it, so I got a go-ahead from him to clean it up for a proper release
 with licenses and stuff (which I think it needs because it's pretty useful
 and should be spread far and wide).

Which is its intended destination extragear-something?

Any reason not to use bugs.kde.org?

The i18n is quite broken, a simple grep gives me
./Config.cpp:39:this-setWindowTitle(KApplication::instance()-
applicationName() + QString( - Configuration));
./Config.cpp:48:QLabel* hotkeyLabel = new QLabel(Shortcut to show 
Mangonel:, this);
./Mangonel.cpp:74:m_actionShow = new KAction(QString(Show Mangonel), 
this);
./Mangonel.cpp:92:QAction* actionConfig = new QAction(KIcon(configure), 
Configuration, this);
./Mangonel.cpp:480:m_descriptionLabel = new QGraphicsTextItem(( + 
application.type + ), this);

Besides all those units in units.c seem untranslatable.

Any reason for not using kunitconversion in there?

Also the units.c copytight header looks a bit scary

Cheers,
  Albert