Re: Telemetry in Plasma and Discover

[Luca Beltrame]

In data martedì 12 luglio 2022 16:52:21 CEST, Nate Graham ha scritto:

Hello Nate,

> +Fabian Vogt and Luca Beltrame specifically
> 
> Thanks, that's a relief! Does this looks legit enough for openSUSE to
> stop patching out the KUSerFeedback integration?

In principle I do not have any objections given the thread, but I'd also like 
input from Christophe (added to CC).  

As an aside I see that similar messages were sent out for PIM and other 
applications. That might be enough to re-enable things everywhere.

-- 
Luca Beltrame - KDE Forums team
GPG key ID: A29D259B

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

D23694: Add support for sshfs to the fstab backend

[Luca Beltrame]

lbeltrame closed this revision.
lbeltrame added a comment.


  
https://invent.kde.org/frameworks/solid/commit/a41ce6a27eb07096356acb3e03ecf69e9ca0173d

REPOSITORY
  R245 Solid

REVISION DETAIL
  https://phabricator.kde.org/D23694

To: lbeltrame, bruns, broulik, fvogt, #kde_connect
Cc: elvisangelaccio, albertvaka, ngraham, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, bruns

D23694: Add support for sshfs to the fstab backend

[Luca Beltrame]

lbeltrame added a comment.


  Any objections? Or I'll merge on Sunday evening UTC+1.

REPOSITORY
  R245 Solid

REVISION DETAIL
  https://phabricator.kde.org/D23694

To: lbeltrame, bruns, broulik, fvogt, #kde_connect
Cc: elvisangelaccio, albertvaka, ngraham, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, bruns

D23694: Add support for sshfs to the fstab backend

[Luca Beltrame]

lbeltrame added a comment.


  @elvisangelaccio Thanks, first of all. If that's the case, do you think it's 
better to touch KIO first before landing this?

REPOSITORY
  R245 Solid

REVISION DETAIL
  https://phabricator.kde.org/D23694

To: lbeltrame, bruns, broulik, fvogt, #kde_connect
Cc: elvisangelaccio, albertvaka, ngraham, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, bruns

D23694: Add support for sshfs to the fstab backend

[Luca Beltrame]

lbeltrame added a comment.


  Would this change be needed on this side or kdeconnect side? I don't mind 
landing this, but I need formal approval by someone. (I won't have any time to 
do additional changes elsewhere).

REPOSITORY
  R245 Solid

REVISION DETAIL
  https://phabricator.kde.org/D23694

To: lbeltrame, bruns, broulik, fvogt, #kde_connect
Cc: elvisangelaccio, albertvaka, ngraham, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, bruns

Re: Move Koko to KDEReview

[Luca Beltrame]

Il giorno Fri, 12 Jun 2020 16:05:36 +
Carl Schwan  ha scritto:


> I asked a few packagers I know and for them, since the packagers can
> download the files and put them into the tarball, it should be fine.

The README should state what purpose those files have. I
didn't find any comment in the CMakeLists that tell what they are
needed for.

Also, under what license do these files fall under?

Please watch build statuses of dependent projects (was: Re: Manner in which kde-gtk-config development is conducted)

[Luca Beltrame]

In data sabato 21 marzo 2020 01:31:40 CET, Ben Cooksley ha scritto:

> Plasma community in general until I raised the matter by commenting on
> the original review - several days after the fact.

I take the opportunity to ask reviewers who review changes to watch for the 
impact of such changes on other dependent projects within KDE. Case in point: 
kaccounts-integration changes (which were reviewed, mind you, so this is not a 
blind push or something reckless)  which broke building / dependent projects 
several times over the course of the past week. 

This is something that even a pre-review CI won't help with, so lxr and 
careful approaches are the way to go. 

-- 
Luca Beltrame - KDE Forums team
GPG key ID: A29D259B

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

D27704: Drop FindAccountsFileDir.cmake

[Luca Beltrame]

lbeltrame requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R155 KAccounts Integration

REVISION DETAIL
  https://phabricator.kde.org/D27704

To: nicolasfella, #frameworks, bshah, leinir, lbeltrame
Cc: lbeltrame

D27704: Drop FindAccountsFileDir.cmake

[Luca Beltrame]

lbeltrame reopened this revision.
lbeltrame added a comment.
This revision is now accepted and ready to land.


  "No one using it" is not correct. Or at least, `ACCOUNTS_SERVICES_DIR`, set 
in that file, is used by kaccounts-providers. I'll revert the change (breaks 
building other projects), but probably it can be fixed if that variable is not 
used anywhere.

REPOSITORY
  R155 KAccounts Integration

REVISION DETAIL
  https://phabricator.kde.org/D27704

To: nicolasfella, #frameworks, bshah, leinir
Cc: lbeltrame

D27633: Port to KPluginLoader

[Luca Beltrame]

lbeltrame added a comment.


  This currently causes a compile error:
  

/home/abuild/rpmbuild/BUILD/kaccounts-integration-20.03.70git.20200225T132248~74e770c/src/daemon/daemon.cpp:62:78:
 error: call of overloaded 'create(AccountsDaemon*, )' is ambiguous

REPOSITORY
  R155 KAccounts Integration

REVISION DETAIL
  https://phabricator.kde.org/D27633

To: nicolasfella, bshah, leinir, #frameworks, apol
Cc: lbeltrame

D27614: build: fix the build where install prefix is not user-writable

[Luca Beltrame]

lbeltrame added a comment.


  +1

REPOSITORY
  R266 Breeze Icons

REVISION DETAIL
  https://phabricator.kde.org/D27614

To: bshah, ngraham, lbeltrame
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

Re: 2 kirigami fixes for a point release

[Luca Beltrame]

In data martedì 18 febbraio 2020 19:26:21 CET, Nate Graham ha scritto:

> Neon is already an OS, whether or not you want to admit it. It's
> installed from an ISO. A hardware vendor (Slimbook) is shipping it on

Erm, where did I say that in my reply? ;)  I merely say that going "Neon or 
else unsupported" is a very downstream-hostile attitude, already seen with 
other software ("my way or the highway" in these cases) such as Audacity or 
Anki (the latter being one of the most downstream-hostile projects I've ever 
known). 

(Neon suffers from the same "LTS problem" for everything that's not KDE-made 
software, FTR, but that's not the issue I want to raise here)


-- 
Luca Beltrame - KDE Forums team
GPG key ID: A29D259B

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

Re: 2 kirigami fixes for a point release

[Luca Beltrame]

In data martedì 18 febbraio 2020 04:03:05 CET, Nate Graham ha scritto:

> think KDE software should be presented to users. Basically, we
> acknowledge that Neon is already an actual OS--the "KDE OS"--and we

Please don't suggest such downstream-hostile solutions, in particular because 
this failing is entirely upstream. We have already plenty in FOSS, I don't 
want KDE to be yet another community that "adopts" them.

"We messed up so let's make things our way" is not an acceptable approach.

-- 
Luca Beltrame - KDE Forums team
GPG key ID: A29D259B

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

D26394: ECMGeneratePriFile: Fix static configurations

[Luca Beltrame]

lbeltrame added subscribers: cgiboudeaux, lbeltrame.
lbeltrame added a comment.


  This breaks stuff in PIM:
  
  [  107s] CMake Error at /usr/share/ECM/modules/ECMGeneratePriFile.cmake:183 
(get_target_property):
  [  107s]   get_target_property() called with non-existent target 
"KF5AlarmCal".

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D26394

To: kfunk, dfaure, winterz, vkrause, apol
Cc: lbeltrame, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, LeGast00n, 
cblack, GB_2, bencreasy, michaelh, ngraham, bruns

D27141: Drop Policykit backend

[Luca Beltrame]

lbeltrame accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R283 KAuth

BRANCH
  no_policykit

REVISION DETAIL
  https://phabricator.kde.org/D27141

To: davidedmundson, lbeltrame
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D26400: Migrate config from KConfig to KConfigXt in order to allow KCM to use it

[Luca Beltrame]

lbeltrame added a reviewer: bruns.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D26400

To: bport, ervin, crossi, meven, #plasma, bruns
Cc: meven, crossi, ervin, kde-frameworks-devel, #baloo, #plasma, hurikhan77, 
lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D26306: Define more documentation search paths (also custom)

[Luca Beltrame]

lbeltrame added a comment.


  +1, would allow us to get rid of some downstream patches.

REPOSITORY
  R238 KDocTools

REVISION DETAIL
  https://phabricator.kde.org/D26306

To: ltoscano
Cc: cgiboudeaux, lbeltrame, rdieter, arojas, rikmills, maximilianocuria, 
asturmlechner, kde-frameworks-devel, kde-doc-english, LeGast00n, gennad, 
fbampaloukas, GB_2, michaelh, ngraham, bruns, skadinna

Re: Blacklisting of PIM from the CI system

[Luca Beltrame]

Il giorno Thu, 05 Dec 2019 01:23:28 +0100
Kevin Kofler  ha scritto:

> The git SHA is not going to work for this, because it is not
> monotonic. What you really want to know is whether you have something
> >= 5.65.0.abcd123, and having a 5.65.0.commithash version is not
> >going to tell you that.

Perhaps (like we do in openSUSE for some git snapshots) using the
number of commits since the last tag? It should increase monotonically.

(might be a bad idea in this context, but I thought I'd throw that into
the mix)

D25106: Also allow invoking session restoration logic when apps are manually launched

[Luca Beltrame]

lbeltrame added a comment.


  Found also by openQA: two kontact windows get opened.

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks, dfaure, vkrause
Cc: lbeltrame, mlaurent, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

Re: Fwd: Plasma 5.17 beta Discover system updates regression

[Luca Beltrame]

In data domenica 22 settembre 2019 14:20:21 CEST, Myriam Schweingruber ha 
scritto:

(this might come in twice, due to gmane seemingly eating a message)

> FYI, this is now reported on both Arch and Neon, with several dozen
> duplicates in this bug report: https://bugs.kde.org/show_bug.cgi?id=411286

According to what I was told, Arch ships Qt 5.13.1 so should be not affected 
by this bug. Can this be confirmed?

So far I know that Neon, and only Neon, is affected due to the Qt version they 
ship, and being a Qt bug, I'd wager it's more of a Neon problem than a KDE 
issue.

openSUSE is not affected because it ships Qt 5.13.x (and Leap has Qt 5.9). 


-- 
Luca Beltrame
GPG key ID:  A29D259B

D24020: Support NetworkManager 1.20 and do actually compile the NM backend

[Luca Beltrame]

lbeltrame added a comment.


  +1

REPOSITORY
  R239 KDELibs4Support

REVISION DETAIL
  https://phabricator.kde.org/D24020

To: arojas
Cc: lbeltrame, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

Re: Ruqola in KDE-review

[Luca Beltrame]

Il giorno Thu, 12 Sep 2019 09:20:45 +0200
laurent Montel  ha scritto:

> Hi,
> I would like to move ruqola to extragear/network.
> So I asked to sysadmin to move ruqola to kde-review (it was done).

Naive question: what does ruqola do? 


pgpQ9_cbUN2Ck.pgp
Description: Firma digitale OpenPGP

D23694: Add support for sshfs to the fstab backend

[Luca Beltrame]

lbeltrame added a comment.


  In D23694#524968 , @fvogt wrote:
  
  > `fuse.sshfs` is used by kdeconnect as well, does that cause some kind of 
conflict?
  
  
  I can't test this in the current network (no office wifi, broadcast blocked 
at all levels). I'll have to see for myself when I get home.

REPOSITORY
  R245 Solid

REVISION DETAIL
  https://phabricator.kde.org/D23694

To: lbeltrame, bruns, broulik, fvogt
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23694: Add support for sshfs to the fstab backend

[Luca Beltrame]

lbeltrame created this revision.
lbeltrame added reviewers: bruns, broulik, fvogt.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
lbeltrame requested review of this revision.

REVISION SUMMARY
  This commit introduces support for sshfs as network filesystem.
  It's probably used enough to warrant the inclusion.

TEST PLAN
  Started dolphin without the patch, did not see mounts in the device list, 
added patch and restarted dolphin, saw the mountpoints in the device list.

REPOSITORY
  R245 Solid

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D23694

AFFECTED FILES
  src/solid/devices/backends/fstab/fstabhandling.cpp

To: lbeltrame, bruns, broulik, fvogt
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D22836: Fix checking dirs for metainfo.yaml with non-ascii chars with Python 2.7

[Luca Beltrame]

lbeltrame accepted this revision.
lbeltrame added a comment.


  Not the prettiest, but I've seen worse. ;)

REPOSITORY
  R264 KApiDox

BRANCH
  handlenonasciipathwpython27

REVISION DETAIL
  https://phabricator.kde.org/D22836

To: kossebau, ochurlaud, bshah, aacid, lbeltrame
Cc: lbeltrame, davidre, aacid, pino, kde-frameworks-devel, kde-doc-english, 
LeGast00n, sbergeron, gennad, fbampaloukas, michaelh, ngraham, bruns, skadinna

D21002: Remove kde4 migration agent completely

[Luca Beltrame]

lbeltrame closed this revision.

REPOSITORY
  R311 KWallet

REVISION DETAIL
  https://phabricator.kde.org/D21002

To: bruns, #frameworks, cfeck, ngraham, aacid
Cc: siddharthasahu, aacid, lbeltrame, kde-frameworks-devel, damjang, LeGast00n, 
sbergeron, michaelh, ngraham, bruns

D21002: Remove kde4 migration agent completely

[Luca Beltrame]

lbeltrame added a comment.


  Should be fixed with ff6b077d9200856dc6e393a04c6b3ae82d9792ed 
.

REPOSITORY
  R311 KWallet

REVISION DETAIL
  https://phabricator.kde.org/D21002

To: bruns, #frameworks, cfeck, ngraham, aacid
Cc: siddharthasahu, aacid, lbeltrame, kde-frameworks-devel, damjang, LeGast00n, 
sbergeron, michaelh, ngraham, bruns

D21002: Remove kde4 migration agent completely

[Luca Beltrame]

lbeltrame reopened this revision.
lbeltrame added a comment.
This revision is now accepted and ready to land.


  This breaks GPG wallets completely. They are no longer accounted for by 
kwalletd: reverting this change makes them work again.

REPOSITORY
  R311 KWallet

REVISION DETAIL
  https://phabricator.kde.org/D21002

To: bruns, #frameworks, cfeck, ngraham, aacid
Cc: siddharthasahu, aacid, lbeltrame, kde-frameworks-devel, damjang, LeGast00n, 
sbergeron, michaelh, ngraham, bruns

D22339: Make sure solid backends are reentrant

[Luca Beltrame]

lbeltrame added a comment.


  Why was this committed when changes were requested? Wasn't that the whole 
point of having reviews?

REPOSITORY
  R245 Solid

REVISION DETAIL
  https://phabricator.kde.org/D22339

To: apol, #frameworks, davidedmundson, bruns
Cc: lbeltrame, bruns, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, 
ngraham

D21004: [UserMetaData] Shortcut attribute queries for the common case

[Luca Beltrame]

lbeltrame accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R824 Baloo Widgets

BRANCH
  bulk_query

REVISION DETAIL
  https://phabricator.kde.org/D21004

To: bruns, #baloo, #frameworks, ngraham, astippich, lbeltrame
Cc: domson, ashaposhnikov, astippich, spoorun, abrahams

D21002: Remove kde4 migration agent completely

[Luca Beltrame]

lbeltrame added a comment.


  +1

REPOSITORY
  R311 KWallet

REVISION DETAIL
  https://phabricator.kde.org/D21002

To: bruns, #frameworks, cfeck, ngraham
Cc: lbeltrame, kde-frameworks-devel, damjang, LeGast00n, michaelh, ngraham, 
bruns

D21313: Create specific directory for kdebugsettings categories file

[Luca Beltrame]

lbeltrame removed a subscriber: cgiboudeaux.
lbeltrame added a reviewer: cgiboudeaux.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D21313

To: mlaurent, dfaure, cgiboudeaux
Cc: kde-frameworks-devel, kde-buildsystem, bencreasy, michaelh, ngraham, bruns

D21313: Create specific directory for kdebugsettings categories file

[Luca Beltrame]

lbeltrame added a subscriber: cgiboudeaux.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D21313

To: mlaurent, dfaure
Cc: cgiboudeaux, kde-frameworks-devel, kde-buildsystem, bencreasy, michaelh, 
ngraham, bruns

D20659: Detach container in Component::cleanUp before interating

[Luca Beltrame]

lbeltrame added a comment.


  I confirm there are no more issues in valgrind after adding this patch.

REPOSITORY
  R268 KGlobalAccel

REVISION DETAIL
  https://phabricator.kde.org/D20659

To: fvogt, #frameworks
Cc: lbeltrame, kde-frameworks-devel, michaelh, ngraham, bruns

D20659: Detach container in Component::cleanUp before interating

[Luca Beltrame]

lbeltrame added a reviewer: Frameworks.

REPOSITORY
  R268 KGlobalAccel

REVISION DETAIL
  https://phabricator.kde.org/D20659

To: fvogt, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D20594: Fix MobileTextSelection namespacing

[Luca Beltrame]

lbeltrame accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D20594

To: broulik, #plasma, mart, hein, lbeltrame
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

Re: liquidshell in kdereview

[Luca Beltrame]

Il giorno Sun, 14 Apr 2019 16:43:54 +0200
Martin Koller  ha scritto:

> The cmake file is the ultimate source for specifying what the
> application depends on. Adding this somewhere else will easily get

READMEs can be useful for packagers instead of manually watching the
output of CMake or reading CMakelists.txt (fine for simpler projects,
but not for large ones).


-- 
Luca Beltrame
GPG key ID: A29D259B


pgpJP4dfjoxjC.pgp
Description: Firma digitale OpenPGP

D20509: Detect duplicate ANDROID_EXTRA_LIBS and minor bug fix

[Luca Beltrame]

lbeltrame added reviewers: apol, vkrause.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D20509

To: sh-zam, apol, vkrause
Cc: kde-buildsystem, kde-frameworks-devel, bencreasy, michaelh, ngraham, bruns

D20465: [KDynamicJobTracker] When kuiserver isn't available, also fall back to widget dialog

[Luca Beltrame]

lbeltrame added a comment.


  +1.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20465

To: broulik, #frameworks, dfaure, lbeltrame
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 10:17:18 CET, Tomaz Canabrava ha scritto:

> The problem is that a git comit is a git commit, there's no way that a
> typo will be treated differently then another commit.

It's a "social" problem and not a technical one: you can see it across 
repositories managed by different groups. Some are very used to reviews, 
others not (and not necessarily PIM).

Hence the (even debatable, if you may) proposal.

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 15:15:23 CET, Nate Graham ha scritto:

> In this case, it seems like the problem is that there are certain
> individuals or teams that are pushing risky, breaking changes without code
> review, and then ignoring failures in the CI. I think we might do well to
> try to answer the question of why that's happening before we create a new
> rule aimed at stopping it.

Well put. Review or not review, clearly something in the process has failed, 
and I suspect "friction" rather than actual bad-faith ignorance of the CI 
results. 

So, perhaps to force myself out of the bikeshedding (mea culpa) on reviews, 
I'll steal Friedrich's points from another mail and put them here 
synthetically:

- Why are the CI mails ignored / filtered? Too many, hard to parse, difficult 
to interpret?
- What can be done to have people look at them and make sure they don't 
overlook breakage?

At least for the second point, as I mentioned earlier in the thread, probably 
having the committer being mailed in case of failure (GitLab does this IIRC) 
might be better than just on the mailing list. The second approach is what we 
use in openSUSE, where I usually don't subscribe to failure notifications 
(almost 300 packages is overkill) but a bot starts pestering me with mails the 
moment build failures go unfixed (granted, the time scale is different). 

For the first, I'd like people more involved in the development to say their 
word.

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 16:04:01 CET, Boudhayan Gupta ha scritto:

> I don't get why mandatory code reviews are so unpopular.

It's not "unpopular". As far as the discussion goes, the opinions (from 
several parties) say that they're not a silver bullet, and that some projects 
benefit from them more than others. 

The ultimate solution is actually more developers (yeah, I know, easy...).

CI, OTOH, has been IMO very useful (despite the headaches Ben mentions) for 
all the projects in KDE. 

> I don't care if you lose time. I don't want the guys building my house to

You should if the review stays there for years when there's no one else to 
review it. 

> As a user, I simply do not want unreviewed crap running on my computer.

Well, reviews help but they're just part of the equation. CI helps as well 
(and IMO, it should be more visible as I mentioned earlier in the thread).
And perfectly reviewed code (as well as unreviewed code) can still be a 
problem (as an integrator, I see that often). 

> one-liners, you're probably too overconfident to be writing good code
> anyway, so I'm going to operate on the presumption that if the code hasn't
> had more than one pair of eyeballs ever looking at it, it's crap.

I would say that there's no need to be like this. There is bound to be 
disagreement (and there is) but not as much as to define quality on 
assumptions. 

To be clear: I'm neither on the side of "review all the things" nor on the 
anarchist side. I just want to make sure we don't engage in policies that can 
be (potentially, just potentially) harmful for some parts of KDE (while they 
are perfectly OK for others). 

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 10:32:39 CET, Kevin Ottens ha scritto:

> OK, to be fair not 100% today's situation because of the above. It was based
> on best judgment maybe we're missing such a set of guidelines. I admit I'm
> slightly doubtful though.

I can't claim it may work 100%, but I've seen other communities (many, many 
years ago) where a "semi anarchy" replaced by "iron-gripped rules" from one 
day to another actually killed them. 

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 15:15:23 CET, Nate Graham ha scritto:

> In this case, it seems like the problem is that there are certain
> individuals or teams that are pushing risky, breaking changes without code
> review, and then ignoring failures in the CI. I think we might do well to
> try to answer the question of why that's happening before we create a new
> rule aimed at stopping it.

Well put. Review or not review, clearly something in the process has failed, 
and I suspect "friction" rather than actual bad-faith ignorance of the CI 
results. 

So, perhaps to force myself out of the bikeshedding (mea culpa) on reviews, 
I'll steal Friedrich's points from another mail and put them here 
synthetically:

- Why are the CI mails ignored / filtered? Too many, hard to parse, difficult 
to interpret?
- What can be done to have people look at them and make sure they don't 
overlook breakage?

At least for the second point, as I mentioned earlier in the thread, probably 
having the committer being mailed in case of failure (GitLab does this IIRC) 
might be better than just on the mailing list. The second approach is what we 
use in openSUSE, where I usually don't subscribe to failure notifications 
(almost 300 packages is overkill) but a bot starts pestering me with mails the 
moment build failures go unfixed (granted, the time scale is different). 

For the first, I'd like people more involved in the development to say their 
word.

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 15:15:23 CET, Nate Graham ha scritto:

> In this case, it seems like the problem is that there are certain
> individuals or teams that are pushing risky, breaking changes without code
> review, and then ignoring failures in the CI. I think we might do well to
> try to answer the question of why that's happening before we create a new
> rule aimed at stopping it.

Well put. Review or not review, clearly something in the process has failed, 
and I suspect "friction" rather than actual bad-faith ignorance of the CI 
results. 

So, perhaps to force myself out of the bikeshedding (mea culpa) on reviews, 
I'll steal Friedrich's points from another mail and put them here 
synthetically:

- Why are the CI mails ignored / filtered? Too many, hard to parse, difficult 
to interpret?
- What can be done to have people look at them and make sure they don't 
overlook breakage?

At least for the second point, as I mentioned earlier in the thread, probably 
having the committer being mailed in case of failure (GitLab does this IIRC) 
might be better than just on the mailing list. The second approach is what we 
use in openSUSE, where I usually don't subscribe to failure notifications 
(almost 300 packages is overkill) but a bot starts pestering me with mails the 
moment build failures go unfixed (granted, the time scale is different). 

For the first, I'd like people more involved in the development to say their 
word.

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 16:04:01 CET, Boudhayan Gupta ha scritto:

> I don't get why mandatory code reviews are so unpopular.

It's not "unpopular". As far as the discussion goes, the opinions (from 
several parties) say that they're not a silver bullet, and that some projects 
benefit from them more than others. 

The ultimate solution is actually more developers (yeah, I know, easy...).

CI, OTOH, has been IMO very useful (despite the headaches Ben mentions) for 
all the projects in KDE. 

> I don't care if you lose time. I don't want the guys building my house to

You should if the review stays there for years when there's no one else to 
review it. 

> As a user, I simply do not want unreviewed crap running on my computer.

Well, reviews help but they're just part of the equation. CI helps as well 
(and IMO, it should be more visible as I mentioned earlier in the thread).
And perfectly reviewed code (as well as unreviewed code) can still be a 
problem (as an integrator, I see that often). 

> one-liners, you're probably too overconfident to be writing good code
> anyway, so I'm going to operate on the presumption that if the code hasn't
> had more than one pair of eyeballs ever looking at it, it's crap.

I would say that there's no need to be like this. There is bound to be 
disagreement (and there is) but not as much as to define quality on 
assumptions. 

To be clear: I'm neither on the side of "review all the things" nor on the 
anarchist side. I just want to make sure we don't engage in policies that can 
be (potentially, just potentially) harmful for some parts of KDE (while they 
are perfectly OK for others). 

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 16:04:01 CET, Boudhayan Gupta ha scritto:

> I don't get why mandatory code reviews are so unpopular.

It's not "unpopular". As far as the discussion goes, the opinions (from 
several parties) say that they're not a silver bullet, and that some projects 
benefit from them more than others. 

The ultimate solution is actually more developers (yeah, I know, easy...).

CI, OTOH, has been IMO very useful (despite the headaches Ben mentions) for 
all the projects in KDE. 

> I don't care if you lose time. I don't want the guys building my house to

You should if the review stays there for years when there's no one else to 
review it. 

> As a user, I simply do not want unreviewed crap running on my computer.

Well, reviews help but they're just part of the equation. CI helps as well 
(and IMO, it should be more visible as I mentioned earlier in the thread).
And perfectly reviewed code (as well as unreviewed code) can still be a 
problem (as an integrator, I see that often). 

> one-liners, you're probably too overconfident to be writing good code
> anyway, so I'm going to operate on the presumption that if the code hasn't
> had more than one pair of eyeballs ever looking at it, it's crap.

I would say that there's no need to be like this. There is bound to be 
disagreement (and there is) but not as much as to define quality on 
assumptions. 

To be clear: I'm neither on the side of "review all the things" nor on the 
anarchist side. I just want to make sure we don't engage in policies that can 
be (potentially, just potentially) harmful for some parts of KDE (while they 
are perfectly OK for others). 

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 10:32:39 CET, Kevin Ottens ha scritto:

> OK, to be fair not 100% today's situation because of the above. It was based
> on best judgment maybe we're missing such a set of guidelines. I admit I'm
> slightly doubtful though.

I can't claim it may work 100%, but I've seen other communities (many, many 
years ago) where a "semi anarchy" replaced by "iron-gripped rules" from one 
day to another actually killed them. 

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 10:32:39 CET, Kevin Ottens ha scritto:

> OK, to be fair not 100% today's situation because of the above. It was based
> on best judgment maybe we're missing such a set of guidelines. I admit I'm
> slightly doubtful though.

I can't claim it may work 100%, but I've seen other communities (many, many 
years ago) where a "semi anarchy" replaced by "iron-gripped rules" from one 
day to another actually killed them. 

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 10:17:18 CET, Tomaz Canabrava ha scritto:

> The problem is that a git comit is a git commit, there's no way that a
> typo will be treated differently then another commit.

It's a "social" problem and not a technical one: you can see it across 
repositories managed by different groups. Some are very used to reviews, 
others not (and not necessarily PIM).

Hence the (even debatable, if you may) proposal.

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 10:17:18 CET, Tomaz Canabrava ha scritto:

> The problem is that a git comit is a git commit, there's no way that a
> typo will be treated differently then another commit.

It's a "social" problem and not a technical one: you can see it across 
repositories managed by different groups. Some are very used to reviews, 
others not (and not necessarily PIM).

Hence the (even debatable, if you may) proposal.

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 09:50:47 CET, Kevin Ottens ha scritto:
> I'd argue we're loosing more with the current state of PIM than we'd loose
> with mandatory reviews.

Perhaps, instead of an all-or-nothing approach, why not a minimal set of 
"requirements" that would require a review? Yes, it requires more discipline 
from those involved, but at least it will help people getting "ingrained" with 
the concept without being a wall.

Examples:

- No review: typo fixes, compile errors, version bumps (internal)
- Review: build system adjustments (perhaps CC some people knowledgeable in 
this case), non-trivial changes like patches
- "Deprecation" removals (as in the casus belli here) - review if touching 
more than a handful of files / multiple repos

(list made by someone who has a passing knowledge of C++, so feel free to rip 
me to shreds)

Pre-commit CI (i.e. once the switch to GitLab occurs) and perhaps direct 
mailing to the user (as I suggested earlier) in case of continuous failures 
will also help.

If this thing works, one can gradually ramp up the requirements of things that 
go through review when the "muscle memory" is formed.

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 09:50:47 CET, Kevin Ottens ha scritto:
> I'd argue we're loosing more with the current state of PIM than we'd loose
> with mandatory reviews.

Perhaps, instead of an all-or-nothing approach, why not a minimal set of 
"requirements" that would require a review? Yes, it requires more discipline 
from those involved, but at least it will help people getting "ingrained" with 
the concept without being a wall.

Examples:

- No review: typo fixes, compile errors, version bumps (internal)
- Review: build system adjustments (perhaps CC some people knowledgeable in 
this case), non-trivial changes like patches
- "Deprecation" removals (as in the casus belli here) - review if touching 
more than a handful of files / multiple repos

(list made by someone who has a passing knowledge of C++, so feel free to rip 
me to shreds)

Pre-commit CI (i.e. once the switch to GitLab occurs) and perhaps direct 
mailing to the user (as I suggested earlier) in case of continuous failures 
will also help.

If this thing works, one can gradually ramp up the requirements of things that 
go through review when the "muscle memory" is formed.

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 09:29:22 CET, Kevin Ottens ha scritto:

> at your screen or pair with you" in the past. Clearly this compromise gets
> somewhat exploited and that's especially bad in the case of a fragile and
> central component like KDE PIM.

I'm not sure I agree. I can't speak for seasoned developers, but I've found 
myself in a situation (more than once) where the fix is trivial (compile 
error, missing ";", etc) and being forced to go through review would (IMO) 
unnecessarily raise friction.

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 09:29:22 CET, Kevin Ottens ha scritto:

> at your screen or pair with you" in the past. Clearly this compromise gets
> somewhat exploited and that's especially bad in the case of a fragile and
> central component like KDE PIM.

I'm not sure I agree. I can't speak for seasoned developers, but I've found 
myself in a situation (more than once) where the fix is trivial (compile 
error, missing ";", etc) and being forced to go through review would (IMO) 
unnecessarily raise friction.

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

In data giovedì 28 marzo 2019 09:29:22 CET, Kevin Ottens ha scritto:

> at your screen or pair with you" in the past. Clearly this compromise gets
> somewhat exploited and that's especially bad in the case of a fragile and
> central component like KDE PIM.

I'm not sure I agree. I can't speak for seasoned developers, but I've found 
myself in a situation (more than once) where the fix is trivial (compile 
error, missing ";", etc) and being forced to go through review would (IMO) 
unnecessarily raise friction.

-- 
Luca Beltrame
GPG key ID: A29D259B

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

Re: CI system maintainability

[Luca Beltrame]

Il giorno Thu, 28 Mar 2019 19:40:09 +1300
Ben Cooksley  ha scritto:

> Does anyone have any ideas for a long term, proper fix to this?

As I said on IRC, the current subjects for the CI are a little obscure
especially if you do  threading. It'd be better to have a prominent
subject first, like "BUILD FAILURE", which immediately draws attention.

Also, in openSUSE we have a bot that after a certain number of failed
builds it will email the committer / maintainer directly informing them
that the package has not been build successfully in a while. This might
be useful to have in our CI here.


-- 
Luca Beltrame
GPG key ID: A29D259B


pgpvYPwIgkWYv.pgp
Description: Firma digitale OpenPGP

D19698: Remove crash in plasmashell

[Luca Beltrame]

lbeltrame added reviewers: Frameworks, Plasma.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D19698

To: mlaurent, dfaure, #frameworks, #plasma
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D19108: [ExternalExtractor] Provide more helpful output when extractor fails

[Luca Beltrame]

lbeltrame accepted this revision.
lbeltrame added a comment.
This revision is now accepted and ready to land.


  LGTM!. Annoyed me for a long time (and made hard to debug some issues).

INLINE COMMENTS

> externalextractor.cpp:120
>  extractorProcess.start(d->mainPath, QStringList(), QIODevice::ReadWrite);
> +bool started = extractorProcess.waitForStarted();
> +if (!started) {

Will this block anything?

REPOSITORY
  R286 KFileMetaData

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D19108

To: bruns, #baloo, #frameworks, ngraham, poboiko, astippich, lbeltrame
Cc: lbeltrame, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

D19001: katetextbuffer: refactor TextBuffer::save() to better separate code paths

[Luca Beltrame]

lbeltrame added reviewers: dhaumann, cullmann.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D19001

To: mgerstner, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, gennad, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D19001: katetextbuffer: refactor TextBuffer::save() to better separate code paths

[Luca Beltrame]

lbeltrame added a reviewer: KTextEditor.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D19001

To: mgerstner, dhaumann, cullmann, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, gennad, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D18845: authority: add support for passing details to polkit

[Luca Beltrame]

lbeltrame added a reviewer: Frameworks.
lbeltrame added a subscriber: kde-frameworks-devel.

REPOSITORY
  R563 Polkit-1 Qt Library

REVISION DETAIL
  https://phabricator.kde.org/D18845

To: mgerstner, #frameworks
Cc: kde-frameworks-devel

D18952: new find module for Canberra

[Luca Beltrame]

lbeltrame added a reviewer: cgiboudeaux.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D18952

To: sitter, cgiboudeaux
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18947: Bring FindUDev.cmake up to ECM standards

[Luca Beltrame]

lbeltrame added a reviewer: cgiboudeaux.

REPOSITORY
  R245 Solid

REVISION DETAIL
  https://phabricator.kde.org/D18947

To: vkrause, #build_system, cgiboudeaux
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18941: Fix build with cmake 3.5

[Luca Beltrame]

lbeltrame added a reviewer: bruns.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D18941

To: wbauer, #build_system, cgiboudeaux, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D18941: Fix build with cmake 3.5

[Luca Beltrame]

lbeltrame added reviewers: Build System, cgiboudeaux.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D18941

To: wbauer, #build_system, cgiboudeaux
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D18574: Fix various OOB reads and writes in kimg_tga and kimg_xcf

[Luca Beltrame]

lbeltrame added a comment.


  Can you expand a bit the description? I understand you are fixing problems, 
but why the problems are there and what you are doing exactly.

REPOSITORY
  R287 KImageFormats

REVISION DETAIL
  https://phabricator.kde.org/D18574

To: fvogt, aacid
Cc: lbeltrame, kde-frameworks-devel, michaelh, ngraham, bruns

D18527: List Android as officially supported

[Luca Beltrame]

lbeltrame accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R289 KNotifications

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D18527

To: vkrause, lbeltrame
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18424: Instantiate QApplication before KCrash/KCatalog

[Luca Beltrame]

lbeltrame added reviewers: bruns, poboiko.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D18424

To: sdepiets, #baloo, bruns, poboiko
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D18345: Fix python binding generation for classes with deleted copy constructors

[Luca Beltrame]

lbeltrame accepted this revision.
lbeltrame added a comment.
This revision is now accepted and ready to land.


  As far as I understand the logic of the whole thing, it looks sane.  At some 
point we ought to find a way to properly test that the generated code...

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D18345

To: aacid, lbeltrame
Cc: cgiboudeaux, skelly, kde-frameworks-devel, kde-buildsystem, michaelh, 
ngraham, bruns

D17863: Add ecm_check_linker_flags function

[Luca Beltrame]

lbeltrame added a reviewer: cgiboudeaux.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D17863

To: tcberner, #freebsd, dfaure, apol, cgiboudeaux
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D17650: Install kioslave.exe as kioslave5.exe under Windows

[Luca Beltrame]

lbeltrame added a comment.


  I would still suggest you get your stuff done with the binary factory and not 
on the OBS simply because then KDE as a whole can benefit from it. Not doing so 
will make sure that Windows and other platform-specific issues will never be 
found.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D17650

To: habacker, #frameworks, lbeltrame, vonreth
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17650: Install kioslave as kioslave5 on Windows

[Luca Beltrame]

lbeltrame added a comment.


  In D17650#380243 , @habacker wrote:
  
  >
  
  
  
  
  >   $ rpm -q -f /usr/i686-w64-mingw32/sys-root/mingw/bin/kioslave.exe 
  >   mingw32-kdelibs4-4.14.60-30.27.noarch
  >   $ rpm -q -f /usr/i686-w64-mingw32/sys-root/mingw/bin/kioslave.exe 
  >   mingw32-kio-5.47.0-11.28.noarch
  
  For the record, those packages aren't shipped officially by the distribution 
you are using.
  
  >> seeing that it's not possible to upstream patches (and there are quite a 
few needed to keep it building) as the git repo is locked down.
  > 
  > It needs someone to take over maintenance of kdelibs.
  
  No. You need to get over the fact that kdelibs4 development is *done* and 
deal, period. Qt4 is EOL, kdelibs4 is EOL. It is time to move on.
  
  > In recent years, of the 104 KDE4 applications for Windows 
(https://community.kde.org/Windows/Releases/4.10.2), only 7 are available for 
KF5 (https://community.kde.org/Windows, minus the stable versions of Umbrello 
and KMymoney that are still on KDE4).  KDE4 will probably stay as a base for a 
while.
  
  Why not joining the effort of the people that are working to get those 
applications on Windows, rather than relying on unmaintained software? By 
distributing Qt 4.x based software we are exposing users to risks, given it is 
completely unmaintained.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D17650

To: habacker, #frameworks, lbeltrame, vonreth
Cc: heikobecker, ngraham, lbeltrame, kde-frameworks-devel, michaelh, bruns

D17331: Extend PositionCodec unit tests, better code coverage

[Luca Beltrame]

lbeltrame accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R293 Baloo

BRANCH
  submit

REVISION DETAIL
  https://phabricator.kde.org/D17331

To: bruns, #baloo, #frameworks, ngraham, astippich, poboiko, lbeltrame
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D17650: Install kioslave as kioslave5 on Windows

[Luca Beltrame]

lbeltrame added a comment.


  In D17650#379423 , @heikobecker 
wrote:
  
  > In D17650#379396 , @habacker 
wrote:
  >
  > > I think this is unrelated - this request is to fix an issue with an 
available package on a distribution, so can anyone accept this ?
  >
  >
  > I'd argue that the problem is with your distribution. Everybody still 
shipping qt4 is on its own anyway, seeing that it's not possible to
  
  
  It's not even there. I co-maintain the KF5 and Applications packages in 
openSUSE and I would refuse outright such patches downstream (also, there are 
no bug reports downstream either on these possible issues).  For the rest, what 
@heikobecker said is spot on. We don't want extra maintenance to fix *one* edge 
case.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D17650

To: habacker, #frameworks, lbeltrame, vonreth
Cc: heikobecker, ngraham, lbeltrame, kde-frameworks-devel, michaelh, bruns

D17650: Install kioslave as kioslave5 on Windows

[Luca Beltrame]

lbeltrame added a comment.


  In D17650#378765 , @habacker wrote:
  
  > On a recent opensuse Leap 42.3 or 15.x system there is
  >
  > > /usr/lib64/kde4/libexec/kioslave
  > >  /usr/lib64/libexec/kf5/kioslave
  
  
  As a packager of KDE software for said distribution, this is just something 
vestigial due to the kdelibs4.x -> KF5 code, back then when KF5 ported software 
was the minority. At this point in time, such a change does not make sense. If 
you want to push this forward I suggest you find another example aside umbrello 
to use.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D17650

To: habacker, #frameworks, lbeltrame, vonreth
Cc: lbeltrame, kde-frameworks-devel, michaelh, ngraham, bruns

D17650: Install kioslave as kioslave5 on Windows

[Luca Beltrame]

lbeltrame added a reviewer: vonreth.
lbeltrame added a comment.


  That said, let's hear the opinion of someone else as well.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D17650

To: habacker, #frameworks, lbeltrame, vonreth
Cc: lbeltrame, kde-frameworks-devel, michaelh, ngraham, bruns

D17650: Install kioslave as kioslave5 on Windows

[Luca Beltrame]

lbeltrame added a comment.


  > umbrello from binary factory is far from been production ready. Windows 
releases are made from the KDE4 branch. See for example 
https://phabricator.kde.org/T7659
  
  I would argue this is more of umbrello's problem to keep on supporting 
kdelibs 4.x and KF5 at the same time than anything else.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D17650

To: habacker, #frameworks, lbeltrame
Cc: lbeltrame, kde-frameworks-devel, michaelh, ngraham, bruns

D17650: Install kioslave as kioslave5 on Windows

[Luca Beltrame]

lbeltrame added a reviewer: Frameworks.
lbeltrame requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D17650

To: habacker, #frameworks, lbeltrame
Cc: lbeltrame, kde-frameworks-devel, michaelh, ngraham, bruns

D17650: Install kioslave as kioslave5 on Windows

[Luca Beltrame]

lbeltrame added a comment.


  At this point, I'd rather give my -1 to this. The 4.x kdelibs stack is *long* 
unmaintained, as well as Qt. What are real, **compelling** reasons to do this?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D17650

To: habacker
Cc: lbeltrame, kde-frameworks-devel, michaelh, ngraham, bruns

D17090: Use append instead of operator+= when appending to an QVector

[Luca Beltrame]

lbeltrame accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R293 Baloo

BRANCH
  submit

REVISION DETAIL
  https://phabricator.kde.org/D17090

To: bruns, #baloo, #frameworks, ngraham, poboiko, lbeltrame
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D17089: Do not add Type::Document/Presentation/Spreadsheet twice for MS Office docs

[Luca Beltrame]

lbeltrame accepted this revision.
lbeltrame added a comment.
This revision is now accepted and ready to land.


  LGTM; just a minor change needed.

INLINE COMMENTS

> basicindexingjob.cpp:153
> +//{"application/vnd.ms-excel", Type::Document},
> +//{"application/vnd.ms-excel", Type::Spreadsheet},
>  // Office 2007

Remove instead of committing commented out code?

REPOSITORY
  R293 Baloo

BRANCH
  submit

REVISION DETAIL
  https://phabricator.kde.org/D17089

To: bruns, #baloo, #frameworks, ngraham, poboiko, lbeltrame
Cc: lbeltrame, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

Re: Python bindings using cppyy (was: An update on Python bindings)

[Luca Beltrame]

Il giorno Mon, 5 Nov 2018 22:54:40 +0100
Dominik Haumann  ha scritto:

> ... wasn't there also some python related work by Stefan? Or is that
> unrelated?

That involves only the existing bindings, this is completely different
from an architectural viewpoint.

-- 
Luca Beltrame - KDE Forums team
GPG key ID: A29D259B


pgpLnRry67jGG.pgp
Description: Firma digitale OpenPGP

D16523: [Extractor] Replace homegrown IO handler with QDataStream, catch HUP

[Luca Beltrame]

lbeltrame accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R293 Baloo

BRANCH
  extractor

REVISION DETAIL
  https://phabricator.kde.org/D16523

To: bruns, #baloo, #frameworks, ngraham, poboiko, lbeltrame
Cc: lbeltrame, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

D16305: Add a QIconEnginePlugin to allow QIcon deserialization

[Luca Beltrame]

lbeltrame added a comment.


  +1.

REPOSITORY
  R302 KIconThemes

REVISION DETAIL
  https://phabricator.kde.org/D16305

To: fvogt, #frameworks
Cc: lbeltrame, kde-frameworks-devel, michaelh, ngraham, bruns

D16524: [Extractor] Use QDataStream serialization in place of cooked one

[Luca Beltrame]

lbeltrame accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R293 Baloo

BRANCH
  extractor

REVISION DETAIL
  https://phabricator.kde.org/D16524

To: bruns, #baloo, #frameworks, ngraham, poboiko, lbeltrame
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D16523: [Extractor] Replace homegrown IO handler with QDataStream, catch HUP

[Luca Beltrame]

lbeltrame added a comment.


  In general (for my limited Baloo knowledge) this makes sense. You might want 
to add a few CCBUGs if you are aware of specific bugs this alleviates (@ngraham 
or someone from the bugsquad may help).

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D16523

To: bruns, #baloo, #frameworks, ngraham, poboiko
Cc: lbeltrame, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

D16505: [KFileMetaData] Replace QDir::separator() with '/' in unit tests

[Luca Beltrame]

lbeltrame accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R286 KFileMetaData

BRANCH
  qdir_separator

REVISION DETAIL
  https://phabricator.kde.org/D16505

To: bruns, #frameworks, #baloo, lbeltrame
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D16490: [KFileMetaData] Add unittest for XML extractor

[Luca Beltrame]

lbeltrame added a comment.


  Generally looks OK to me, one note on `QDir::separator()` usage.

INLINE COMMENTS

> xmlextractortest.cpp:38
> +{
> +return QLatin1String(INDEXER_TESTS_SAMPLE_FILES_PATH) + 
> QDir::separator() + fileName;
> +}

IIRC you shouldn't use `QDir::separator()`.  See 
http://agateau.com/2015/qdir-separator-considered-harmful/

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16490

To: bruns, #frameworks, astippich
Cc: lbeltrame, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D16255: [Scheduler] Fix wrong usage of obsolete QFileInfo::created() timestamp

[Luca Beltrame]

lbeltrame accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R293 Baloo

BRANCH
  indexer_cleanup

REVISION DETAIL
  https://phabricator.kde.org/D16255

To: bruns, #baloo, #frameworks, poboiko, ngraham, lbeltrame
Cc: lbeltrame, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

D16265: [Scheduler] Use flag to track when a runner is going idle

[Luca Beltrame]

lbeltrame accepted this revision.
lbeltrame added a comment.
This revision is now accepted and ready to land.


  The changes look sane to me. Perhaps wait a couple more days until any other 
objection is raised, then if not, commit away.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D16265

To: bruns, #baloo, #frameworks, poboiko, ngraham, lbeltrame
Cc: lbeltrame, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

D16311: RFC: [KFilePlacesView] Use asynchronous KIO::FileSystemFreeSpaceJob

[Luca Beltrame]

lbeltrame added a comment.


  +1, but I'd like to hear people more experienced than me.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D16311

To: broulik, #frameworks, dfaure, lbeltrame
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16255: [Scheduler] Fix wrong usage of obsolete QFileInfo::created() timestamp

[Luca Beltrame]

lbeltrame added a comment.


  +1, looks fine to me as far as I understand.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D16255

To: bruns, #baloo, #frameworks, poboiko, ngraham
Cc: lbeltrame, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

D15867: Bindings: Remove INSTALL_DIR_SUFFIX from ecm_generate_python_binding

[Luca Beltrame]

lbeltrame accepted this revision.
lbeltrame added a comment.
This revision is now accepted and ready to land.


  Looks OK to me.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D15867

To: bruns, #frameworks, apol, lbeltrame
Cc: lbeltrame, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

Re: RKWard is in kdereview

[Luca Beltrame]

Il giorno Sat, 6 Oct 2018 15:45:39 +0200
Thomas Friedrichsmeier  ha
scritto:

> final step: Today I'd like to ask you to start reviewing RKWard for
> inclusion into exragear (coming from playground).

As a longtime user of rkward (since the KDE 3 times) and as someone who
still uses it for his day to day job I can't comment much on the code
itself but I just want to say: congratulations for reaching this
milestone!

-- 
Luca Beltrame - KDE Forums team
GPG key ID: A29D259B


pgpntzMi7Kceh.pgp
Description: Firma digitale OpenPGP

D15426: Avoid QByteArray::remove in AccessManagerReply::readData

[Luca Beltrame]

lbeltrame added a reviewer: dfaure.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15426

To: fvogt, #frameworks, elvisangelaccio, dfaure
Cc: ngraham, bruns, kde-frameworks-devel, michaelh

D15068: Bindings: Correct handling of sources containing utf-8

[Luca Beltrame]

lbeltrame accepted this revision.
lbeltrame added a comment.
This revision is now accepted and ready to land.


  LGTM.

INLINE COMMENTS

> sip_generator.py:752
>  #
> -return "".join(extract).replace("\n", " ")
> +return b''.join(extract).decode('utf-8').replace("\r\n", " 
> ").replace("\n", " ").replace("\r", " ")
>  

Perhaps mention in the comment that you're doing this for all the types of 
newlines?

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D15068

To: bruns, #frameworks, lbeltrame
Cc: lbeltrame, bcooksley, jtamate, kde-frameworks-devel, kde-buildsystem, 
michaelh, ngraham, bruns

D15113: Add license text of GPL 2.0, LGPL 2.0 and LGPL 2.1

[Luca Beltrame]

lbeltrame added a comment.


  +1

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D15113

To: fvogt, #frameworks
Cc: lbeltrame

D14395: [KSambaShare] Check file that's changed before reloading

[Luca Beltrame]

lbeltrame added a comment.


  +1

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D14395

To: broulik, #frameworks, dfaure
Cc: lbeltrame, kde-frameworks-devel, michaelh, ngraham, bruns

Re: Upcoming change to mail infrastructure -> SPF still broken

[Luca Beltrame]

Il giorno Wed, 4 Jul 2018 14:45:00 +0200
Reindl Harald  ha scritto:

> https://bugs.kde.org/show_bug.cgi?id=392685
> 2018-04-03 18:45:47 UTC

For the record, Sysadmin tickets should be filed through
Phabricator (https://go.kde.org/systickets) rather than Bugzilla: this
makes sure that they won't get lost.

(Bugzilla is still used for KDE software - this is for Sysadmin-related
stuff).


pgpQx5dbmHCoo.pgp
Description: Firma digitale OpenPGP