Re: KDEReview for kio-s3
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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 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
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
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
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
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
> 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
On 28 September 2016 at 10:17, Ben Cooksleywrote: > 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
On Wed, Jun 21, 2017 at 5:06 PM, Jonathan Riddellwrote: > 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
-- 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
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 Cidwrote: > > 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
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
On Mon, Jun 19, 2017 at 11:42 PM, Albert Astals Cidwrote: > 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
On 16 June 2017 at 16:07, Yichao Yuwrote: > 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
> 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
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
On Thu, May 18, 2017 at 11:08 AM, Marco Martinwrote: > 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
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
On Sun, May 21, 2017 at 7:16 PM, Albert Astals Cidwrote: > 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
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
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
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
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 Grulichwrote: > > 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
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 Grulichwrote: > 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
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
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
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
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
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
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 Grulichwrote: > > > 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
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
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 Grulichwrote: > > > > 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
On Tue, May 2, 2017 at 3:09 PM, Jan Grulichwrote: > 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
On Tue, May 2, 2017 at 12:36 PM, Jan Grulichwrote: > 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
On úterý 2. května 2017 14:19:04 CEST Elvis Angelaccio wrote: > On Tue, May 2, 2017 at 12:36 PM, Jan Grulichwrote: > > 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
On úterý 2. května 2017 12:21:31 CEST Aleix Pol wrote: > On Tue, May 2, 2017 at 12:15 PM, Albert Astals Cidwrote: > > 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
On Tue, May 2, 2017 at 12:15 PM, Albert Astals Cidwrote: > 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
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
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
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
Ben Cooksley ha scritto: > On Sat, Oct 1, 2016 at 10:49 AM, Albert Astals Cidwrote: >> 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
On Sat, Oct 1, 2016 at 10:49 AM, Albert Astals Cidwrote: > 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
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 Cidwrote: > > 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
On Fri, Sep 30, 2016 at 11:27 AM, Albert Astals Cidwrote: > 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
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
On Wed, Sep 28, 2016 at 9:06 AM, Allen Winterwrote: > 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
Il 27 settembre 2016 22:06:35 CEST, Allen Winterha 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
написане 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
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
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