D20086: Fix window height of Screen Locking KCM

[David Edmundson]

davidedmundson added a comment.


  Hardcoding pixel size won't impact scaling.
  
  ---
  
  You will never get it spot on two rows with any combination of pixel size or 
units or whatever you add here. The correct size is dependent on the exact 
children.
  The only way you'll get it to be correct is to bind this stack view to 
currentItem.implicitHeight and making sure that the implicit size propagates 
upwards correctly.

REPOSITORY
  R133 KScreenLocker

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

To: tigrang, davidedmundson
Cc: abetts, filipf, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

Re: CI system maintainability

[laurent Montel]

Le jeudi 28 mars 2019, 18:27:42 CET Friedrich W. H. Kossebau a écrit :
> Thanks for reply. More below:
> 
> Am Donnerstag, 28. März 2019, 16:56:33 CET schrieb laurent Montel:
> > Le jeudi 28 mars 2019, 16:11:12 CET Friedrich W. H. Kossebau a écrit :
> > > Hi Laurent,
> > > 
> > > Am Donnerstag, 28. März 2019, 14:33:59 CET schrieb laurent Montel:
> > > > For example I works all days on kde (pim or other) when I wake up, or
> 
> at
> 
> > > > noon after my lunch or the evening, I will not wait several days for a
> > > > review because nobody has time to do it.
> > > > 
> > > > (For example I make ~ 15 commits by days on pim/ruqola/framework, I
> > > > don't
> > > > want to wait several days/weeks until someone wants to review my
> 
> patchs)
> 
> > > Something might be lost in translation here, do you think, because you
> > > work
> > > daily on code of KDE projects, and other people (so potential reviewers)
> > > do
> > > not, this is an argument to do instant pushes of unreviewed commits?
> > 
> > I maintain pim* from several years, I fix bugs, I improve apps, I fix all
> > bugs that I put in my code, so for this one I consider that I can commit
> > without review them (as other guys on other project that they maintain).
> > For framework I mainly use phabricator.
> 
> I was thinking of projects where there are multiple persons contributing/
> maintaining, like Frameworks.

For framework I use mainly phabricator
See https://phabricator.kde.org/people/revisions/196/


> 
> So for projects where you are the only person who has any real insight,
> indeed I agree to pushing directly, as I do that as well :)
> 
> Because IMHO the costs for having to hope & wait for somebody else to do a
> "review", where one then spends lots of time rather explaining things to
> someone, who is not really into the project and also never might be, is not
> anywhere worth the time one could have otherwise invested in fixing other
> existing bugs or adding new features or improving code quality.
> 
> If people want to get into a project, doing own patches or just watching the
> commits and asking normally on irc or by email about the architecture
> should work good enough. Abusing reviews for teaching about some project
> would annoy me at least, usually the patch is to fix some annoying bug or
> add a wanted feature, so it should get in as early as possible.
> 
> > > Not sure where this is from, but often I have seen an unwritten policy
> > > applied where people for a patch uploaded for review after one week of
> > > no
> > > response add a ping and then wait another week, before finally pushing
> 
> the
> 
> > > change. To me this seems a fair and reasonable policy, only ignores
> 
> people
> 
> > > who are on vacation for some more weeks or otherwise inactive, but I
> > > have
> > > not seen that ever been a real issue.
> > 
> > 2 weeks for a commit, for me it's too long.
> > I understand why people can be demotivated when they must wait long time
> > until having an answer.
> 
> Well, 2 weeks is the time-out, only reached in worst case. Ideally people
> give some feedback before, which often enough happens. And indeed one also
> has to hunt people down to get a review, like at meetings or in chat (or
> trade review work with each other :) ). But isn't this the same also at
> work- work, unless there is a dedicated review team which needs to react by
> itself? Co-operating on something needs social interaction after all.
> 
> > > Given the actual purpose of this thread, I would be more curious how you
> > > have CI integrated in your workflow?
> > 
> > CI: sometime I look at it, sometime not, sometime some guys informs me
> > that
> > it's broken (I remember that Luca told me some days ago that a package
> > didn't compile, so I fixed it).
> > Sometime my code compiles on local so for me it's ok but it's just that I
> > forgot to trash my builddir.
> 
> So you do not go on yourself to build.kde.org and check yourself? Does #kde-
> pim have a bot reporting build failures?

Long time ago we had it in #kontact.
It's not the case now.

> 
> For what I can tell e.g. for KDevelop, personally I rely on the bot on
> #kdevelop  reporting the CI state/build results, as I only look at emails
> rather once a day, while the chat channel is more real-time, and one also
> can immediately talk to peers about why some build now fails, as well as
> coordinate who will fix that.
> 
> > > And where things could be improved, to
> > > prevent the current state of unhappiness for people who care about CI
> 
> some
> 
> > > more? Given you said you work daily on KDE projects, it seems that the
> > > brokeness of those projects on the KDE CI has slipped your attention. So
> > > how does this happen, and how could this be prevented, other than people
> > > having to hunt you down on irc and tell you :)
> > 
> > I think that Luca idea to send an email directly to the people which
> > breaks
> > code when it breaks from several commit is a good idea.
> 

D19822: [Task Manager] Toggle mute when the audio indicator is clicked

[Farid Boudedja]

faridb updated this revision to Diff 55003.
faridb added a comment.


  - Revert "[Task Manager] Make mute/unmute behaviour configurable"
  
  In D19822#439724 , @hein wrote:
  
  > Back to work: For this patch to proceed further, please remove the setting.
  
  
  I removed the configuration option.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19822?vs=54823=55003

BRANCH
  taskmanager-mute (branched from master)

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

AFFECTED FILES
  applets/taskmanager/package/contents/ui/Task.qml

To: faridb, #plasma, #vdg, ngraham, filipf, hein
Cc: mart, hein, filipf, mvourlakos, ngraham, broulik, plasma-devel, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

Re: CI system maintainability

[Alexander Neundorf]

Hi,

On 2019 M03 28, Thu 16:04:01 CET Boudhayan Gupta wrote:
> Hi,
> 
> On Thu, 28 Mar 2019 at 15:21, Kevin Ottens  wrote:
> > Hello,
> > 
> > On Thursday, 28 March 2019 14:33:59 CET laurent Montel wrote:
> > > I am against to force mandatory review, as it will create a lot of lose
> > 
> > of
> > 
> > > time,
> > 
> > As I said, unpopular.
> 
> I don't get why mandatory code reviews are so unpopular.
> 
> I don't care if you lose time. 

Oh, yes, you do.
Assuming a developers would severly loose time due to waiting for reviews, at 
some point motivation would decline, at some point contributions and progress 
would decline...

> I don't want the guys building my house to
> cut corners mixing my concrete because it's going to save time. 

I would certainly be happy enough if an experienced guy would build my house 
in a reasonable time relatively quickly, more happy than if he would have to 
wait for some authority to check hs work after every day, and so make almost 
no progress anymore...
(I'm not saying that mandatory reviews would have that effect, I just think 
the argument is wrong).

> Why are you
> in such a massive hurry to make changes to software which for example holds
> access to my Google Account password? In fact, the very fact that you make
> this argument makes me wonder if I'm running trustable code on my computer
> at all, 

Now, when you raise the question whether kmail+akonadi is "trustable code", 
the ice is getting thin...
I think not having mandatory code reviews for akonadi + kde pim is not the 
problem in the case of kdepim.

...
> As a user, I simply do not want unreviewed crap running on my computer.
> Yes, crap, because no software engineer writes bug-free code all the time,
> and if you're so overconfident that you don't need reviews on even your
> 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.

If you put it that strong, it's wrong.
Code which has not been reviewed is not generally "crap". Code which has been 
reviewed is not generally "high quality".
Unreviewed code can be good, and often is good, and reviews, maybe especially 
if they are mandatory, *can* be crappy: just pointing out formatting issues, 
Oking the patch without understanding the big picture (and feeling somewhat 
pressured to accept a patch because the review is mandatory and otherwise you 
are blocking the other developer, etc.)

> As a developer, I know that even one-liners, and especially one-liners, the
> sort where you think "meh, this is a tiny little thing, I don't have to be
> careful" are the ones that have the most dangerous typos and unintended
> bugs.

That's also a wrong argument. one-liners are not especially prone to causing 
most bugs. They may introduce bugs, but I think, since they are small, this is 
less likely than for bigger patches.

...
> In a project like PIM, if the code hasn't been through review, which
> independent party do I trust to verify that you're not, for example,
> leaking my Google password to some world-readable tempfile? 

Having mandatory reviews for a central and complex component like akonadi 
looks like a very good and obvious idea.
OTOH if there is only one developer who is really expert for akonadi, this 
makes it kind of unfeasible.
IMO this specific case is also a technical issue. Akonadi makes things 
complicated (and it's already 13 years old, so it should be mature in the 
meantime).

...
> Let me be very clear - even if you're the best damn programmer on the
> planet, if *you* wrote the code, I do not trust *you* one inch to tell me
> that that code is correct. That verification needs to come from someone
> else, someone who does not have a conflict of interest in seeing that code
> get into production.

Sorry, your arguments are too black & white.

Alex

Re: CI system maintainability

[Dr.-Ing. Christoph Cullmann]

Hi,

> Hi,
> 
> On Thu, 28 Mar 2019 at 15:21, Kevin Ottens  wrote:
> 
>> Hello,
>>
>> On Thursday, 28 March 2019 14:33:59 CET laurent Montel wrote:
>> > I am against to force mandatory review, as it will create a lot of lose
>> of
>> > time,
>>
>> As I said, unpopular.
>>
> 
> I don't get why mandatory code reviews are so unpopular.
> 
> I don't care if you lose time. I don't want the guys building my house to
> cut corners mixing my concrete because it's going to save time. Why are you
> in such a massive hurry to make changes to software which for example holds
> access to my Google Account password? In fact, the very fact that you make
> this argument makes me wonder if I'm running trustable code on my computer
> at all, because apparently doing it quickly is far more important than
> doing it right.
> 
> As a user, I simply do not want unreviewed crap running on my computer.
> Yes, crap, because no software engineer writes bug-free code all the time,
> and if you're so overconfident that you don't need reviews on even your
> 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.
> 
> As a developer, I know that even one-liners, and especially one-liners, the
> sort where you think "meh, this is a tiny little thing, I don't have to be
> careful" are the ones that have the most dangerous typos and unintended
> bugs. Reviews catch that.
> 
> In a project like PIM, if the code hasn't been through review, which
> independent party do I trust to verify that you're not, for example,
> leaking my Google password to some world-readable tempfile? Do you really
> expect every user to read the entire codebase for themselves and make sure
> that's not being done? The whole point of having all the code out in the
> open for independent audit purposes, to protect your security and privacy
> and what not is completely moot if no one else actually looks at the code
> anyway. And let's be honest, the code quality of some of KDE's projects - I
> wouldn't touch them with a six-foot pole. The ones I would touch though,
> all have multiple people looking at the code and reviewing everything that
> goes in.
> 
> Let me be very clear - even if you're the best damn programmer on the
> planet, if *you* wrote the code, I do not trust *you* one inch to tell me
> that that code is correct. That verification needs to come from someone
> else, someone who does not have a conflict of interest in seeing that code
> get into production. This is nothing personal, this is confirmation bias on
> the author's part which leads to issues that even though they might be
> infrequent, usually have catastrophic impact.
> 
> And if "culture" trumps over engineering best practices, it follows that I
> should just stop using software produced by this entity because who knows
> what it's doing.

Mandatory code reviews are nice, if you have the manpower.

Look at our phabricator, look for example at the queue of reviews for 
KTextEditor.

The team is small and the code is complex and old (not rocket-science, it is a 
text editor,
but still...).

I and others tried to get more reviews done in the past, but actually I merged 
more
than once stuff that I reviewed but it did break the CI.

In most cases I fixed it myself afterwards or reverted again, but a few times
I needed to get ping'd by others that I was stupid, too.

In the current case discussed an error happend, it could have happend exactly 
the same
way if for example "me" would have reviewed it.

A lot of the changes which are at the moment in the queue are stuck because

a) lack of time to review the change
b) lack of time to ever be able to test the change

For any non-trivial behavior change (especially for features you not use 
yourself at all),
any meaningful review is a full-time job. e.g. in our company you would let 
some student
test the changed behavior some days. This is just not feasible for me, and yes,
for some of these changes, rather than abandoning them (and trashing precious 
work others did),
I will somewhen apply them with close to zero real testing.

Greetings
Christoph

-- 
- Dr.-Ing. Christoph Cullmann -
AbsInt Angewandte Informatik GmbH  Email: cullm...@absint.com
Science Park 1 Tel:   +49-681-38360-22
66123 Saarbrücken  Fax:   +49-681-38360-20
GERMANYWWW:   http://www.AbsInt.com

Geschäftsführung: Dr.-Ing. Christian Ferdinand
Eingetragen im Handelsregister des Amtsgerichts Saarbrücken, HRB 11234

Re: CI system maintainability

[Dominik Haumann]

Kevin Ottens  schrieb am Do., 28. März 2019, 09:29:

> Hello,
>
> On Thursday, 28 March 2019 09:16:11 CET Ben Cooksley wrote:
> > Please note that the commits in this instance were pushed without
> > review, so restrictions on merge requests wouldn't make a difference
> > in this case unfortunately.
>
> Maybe it's about time to make reviews mandatory...


We could make it mandatory with a possible backdoor. Like if you have a
line in your commit log saying "I know what I am doing." then the commit
could be done without review. Of course this sounds like everyone could use
this backdoor always, but I doubt this would be the case.

Also, unreviewed commits could have a "[not reviewed]" suffix on
kde-comm...@kde.org like the License additions so that it's easier to spot
unreviewed commits. We could even CC the author or respective mailing list
recommending that reviews should be done for future commits.

Just some thoughts :)

Greetings
Dominik


I know it's unpopular in
> KDE, and I advocated for "don't force a tool if you can get someone to
> look 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.
>
> Regards.
> --
> Kevin Ottens, http://ervin.ipsquad.net
>

D20086: Fix window height of Screen Locking KCM

[Tigran Gabrielyan]

tigrang added a comment.


  @filipf Here's a diff with units.gridUnit 
https://phabricator.kde.org/differential/diff/54998/ which doesn't quite work 
out to 2 rows for every scaling factor but is a bit better than hard-coded 
pixel size. I'm not sure how to do doubling the height of one grid item.

REPOSITORY
  R133 KScreenLocker

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

To: tigrang, davidedmundson
Cc: abetts, filipf, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

Re: CI system maintainability

[Friedrich W. H. Kossebau]

Thanks for reply. More below:

Am Donnerstag, 28. März 2019, 16:56:33 CET schrieb laurent Montel:
> Le jeudi 28 mars 2019, 16:11:12 CET Friedrich W. H. Kossebau a écrit :
> > Hi Laurent,
> > 
> > Am Donnerstag, 28. März 2019, 14:33:59 CET schrieb laurent Montel:
> > > For example I works all days on kde (pim or other) when I wake up, or 
at
> > > noon after my lunch or the evening, I will not wait several days for a
> > > review because nobody has time to do it.
> > > 
> > > (For example I make ~ 15 commits by days on pim/ruqola/framework, I
> > > don't
> > > want to wait several days/weeks until someone wants to review my 
patchs)
> > 
> > Something might be lost in translation here, do you think, because you
> > work
> > daily on code of KDE projects, and other people (so potential reviewers)
> > do
> > not, this is an argument to do instant pushes of unreviewed commits?
> 
> I maintain pim* from several years, I fix bugs, I improve apps, I fix all
> bugs that I put in my code, so for this one I consider that I can commit
> without review them (as other guys on other project that they maintain).
> For framework I mainly use phabricator.

I was thinking of projects where there are multiple persons contributing/
maintaining, like Frameworks.

So for projects where you are the only person who has any real insight, 
indeed I agree to pushing directly, as I do that as well :)

Because IMHO the costs for having to hope & wait for somebody else to do a 
"review", where one then spends lots of time rather explaining things to 
someone, who is not really into the project and also never might be, is not 
anywhere worth the time one could have otherwise invested in fixing other 
existing bugs or adding new features or improving code quality.

If people want to get into a project, doing own patches or just watching the 
commits and asking normally on irc or by email about the architecture should 
work good enough. Abusing reviews for teaching about some project would annoy 
me at least, usually the patch is to fix some annoying bug or add a wanted 
feature, so it should get in as early as possible.

> > Not sure where this is from, but often I have seen an unwritten policy
> > applied where people for a patch uploaded for review after one week of no
> > response add a ping and then wait another week, before finally pushing 
the
> > change. To me this seems a fair and reasonable policy, only ignores 
people
> > who are on vacation for some more weeks or otherwise inactive, but I have
> > not seen that ever been a real issue.
> 
> 2 weeks for a commit, for me it's too long.
> I understand why people can be demotivated when they must wait long time
> until having an answer.

Well, 2 weeks is the time-out, only reached in worst case. Ideally people 
give some feedback before, which often enough happens. And indeed one also 
has to hunt people down to get a review, like at meetings or in chat (or 
trade review work with each other :) ). But isn't this the same also at work-
work, unless there is a dedicated review team which needs to react by itself? 
Co-operating on something needs social interaction after all. 

> > Given the actual purpose of this thread, I would be more curious how you
> > have CI integrated in your workflow?
> 
> CI: sometime I look at it, sometime not, sometime some guys informs me that
> it's broken (I remember that Luca told me some days ago that a package
> didn't compile, so I fixed it).
> Sometime my code compiles on local so for me it's ok but it's just that I
> forgot to trash my builddir.

So you do not go on yourself to build.kde.org and check yourself? Does #kde-
pim have a bot reporting build failures?

For what I can tell e.g. for KDevelop, personally I rely on the bot on 
#kdevelop  reporting the CI state/build results, as I only look at emails 
rather once a day, while the chat channel is more real-time, and one also can 
immediately talk to peers about why some build now fails, as well as 
coordinate who will fix that.

> > And where things could be improved, to
> > prevent the current state of unhappiness for people who care about CI 
some
> > more? Given you said you work daily on KDE projects, it seems that the
> > brokeness of those projects on the KDE CI has slipped your attention. So
> > how does this happen, and how could this be prevented, other than people
> > having to hunt you down on irc and tell you :)
> 
> I think that Luca idea to send an email directly to the people which breaks
> code when it breaks from several commit is a good idea.

Noted. Personally I would also fancy that over the generic mass emailing, 
where most of the time it was somebody else breaking stuff, so they should 
care. Given the time-offset due to build times it is also unclear who broke 
things, given the email is not easy to parse, and one might already be off 
again to other things in life.

Question is: when would you read the email, and how quick would you react?

One could say, Ben has had 

Re: CI system maintainability

[Friedrich W. H. Kossebau]

Am Donnerstag, 28. März 2019, 16:04:01 CET schrieb Boudhayan Gupta:
> I don't care if you lose time. I don't want the guys building my house to
> cut corners mixing my concrete because it's going to save time.

There is a difference here though, no? The people building your house will 
not live in that house. They only make money from building, so: less effort & 
lesser time for same money received -> better.

We here create software we use ourselves, no? So we are building our own 
house, and would not be expected to cut corners on our own grounds.

> As a user, I simply do not want unreviewed crap running on my computer.
> Yes, crap, because no software engineer writes bug-free code all the time,
> and if you're so overconfident that you don't need reviews on even your
> 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.

Hmmm... (I cannot stop myself typing the following :) )

In that case, I have to immediately notify you to make sure that you remove 
Okteta from any of the devices you have reach to, if you even ever installed 
it, best recommend your distribution to delete the package. Because 
shockingly all the >4000 commits of its >100k lines of code have been done 
without review. So it surely is an insane pile of crap by presumption, unless 
finally someone will give it another pair of eyeballs.

Though I assume no-one is using it anyway, given there are so few bugs 
reported ;)

> As a developer, I know that even one-liners, and especially one-liners, the
> sort where you think "meh, this is a tiny little thing, I don't have to be
> careful" are the ones that have the most dangerous typos and unintended
> bugs. Reviews catch that.

This sounds to me like review is magically preventing bugs.
Well, it increases the chance things get catched. Though reviews also 
increase the chance to be sloppy, as there is: review to catch that. Then 
after all is the reviewer also a developer, who also only sees a one-liner, 
where they think "meh, this is a tiny little thing, I don't have to be 
careful".

A review is only useful if the reviewers are qualified and invest the proper 
resources.

I prefer one responsible experienced developer over an unresponsible 
unexperienced developer & unresponsible unexperienced reviewer any time.

More I prefer of course one responsible experienced developer & one 
responsible experienced reviewer :)

Based on my personal experience bubble, not by any scientific means.

Cheers
Friedrich

Re: CI system maintainability

[Volker Krause]

On Thursday, 28 March 2019 16:11:12 CET Friedrich W. H. Kossebau wrote:
> Am Donnerstag, 28. März 2019, 14:33:59 CET schrieb laurent Montel:
> > For example I works all days on kde (pim or other) when I wake up, or at
> > noon after my lunch or the evening, I will not wait several days for a
> > review because nobody has time to do it.
> > 
> > (For example I make ~ 15 commits by days on pim/ruqola/framework, I don't
> > want to wait several days/weeks until someone wants to review my patchs)
> 
> Something might be lost in translation here, do you think, because you work
> daily on code of KDE projects, and other people (so potential reviewers) do
> not, this is an argument to do instant pushes of unreviewed commits?
> 
> While I understand one can get impatient if not getting instant review of
> changes one would like to depend on with further changes (I know this well
> :) ), still this seems a flawed argument at least for
> part-time-contributors based KDE projects, where the people one co-operates
> with only have time now and then, like once per week. It could be seen
> unfair & ignorant to them if one simply ignores their opinion, because one
> has more time reserved/ available.

I don't think any of that was meant here. The scenario that Laurent has in 
mind I think, and that I'm facing too, is that putting up a few dozen patches 
a week in a single repository for review and then having to wait for the 
review timeout because there's nobody else working on it is not even remotely 
practical, let alone with the current toolset of arc/phab.

> Not sure where this is from, but often I have seen an unwritten policy
> applied where people for a patch uploaded for review after one week of no
> response add a ping and then wait another week, before finally pushing the
> change. To me this seems a fair and reasonable policy, only ignores people
> who are on vacation for some more weeks or otherwise inactive, but I have
> not seen that ever been a real issue.

This works fine if you have less than a handful of patches in a single repo, 
and people actually review things. And we make plenty of use of that:
https://phabricator.kde.org/people/revisions/196/
https://phabricator.kde.org/people/revisions/208/

In fact I was just criticized last weekend at the privacy sprint for sending 
too many reviews ;-)

Regards,
Volker

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

Re: CI system maintainability

[Harald Sitter]

On Thu, Mar 28, 2019 at 3:57 PM David Jarvie  wrote:
> I agree. Mandatory reviews might work if there is a team of active people 
> working on a project, but if there is only one person with real knowledge of 
> the code

We do have common ownership of code, so if there is only one person
with real knowledge of the code that is a problem in of itself... A
problem which really should get solved... For example by having
mandatory reviews so someone actually has to review code they know
just as little about as everyone else, so in turn they now know a bit
more and can more confidently do reviews ;)

Now to be sure, I am not certain mandatory reviews are in fact the
answer to the problem at hand, nor if they would in fact be possible
to implement reliable. From personal experience I'll say that reviews
almost always are worth it, even for the simple typo fixes. And I also
almost find someone to give at least a casual review even when they
know nothing of the code base. Sometimes perhaps even "I couldn't say
if this change is good, but the code looks correct at least +1" is
better than nobody having looked at the code at all. It ultimately
also becomes a matter of busfactor. If nobody ever has reason to look
at code with a single principal author the busfactor will never
improve.

HS

Re: CI system maintainability

[Volker Krause]

On Thursday, 28 March 2019 16:32:34 CET Luca Beltrame wrote:
> 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.

That "risky breaking change" was a 5 line fix for a Qt 5.13 build issue that 
had been successfully deployed to multiple repos before. The failure was also 
not ignored but noticed and fixed within about an hour, just not in all 
affected branches.

> 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.

See my earlier mail in this thread on how this slipped through for me.

Regards,
Volker


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

Re: CI system maintainability

[laurent Montel]

Le jeudi 28 mars 2019, 16:11:12 CET Friedrich W. H. Kossebau a écrit :
> Hi Laurent,
> 
> Am Donnerstag, 28. März 2019, 14:33:59 CET schrieb laurent Montel:
> > For example I works all days on kde (pim or other) when I wake up, or at
> > noon after my lunch or the evening, I will not wait several days for a
> > review because nobody has time to do it.
> > 
> > (For example I make ~ 15 commits by days on pim/ruqola/framework, I don't
> > want to wait several days/weeks until someone wants to review my patchs)
> 
> Something might be lost in translation here, do you think, because you work
> daily on code of KDE projects, and other people (so potential reviewers) do
> not, this is an argument to do instant pushes of unreviewed commits?

I maintain pim* from several years, I fix bugs, I improve apps, I fix all bugs 
that I put in my code, so for this one I consider that I can commit without 
review them (as other guys on other project that they maintain).
For framework I mainly use phabricator.

> 
> While I understand one can get impatient if not getting instant review of
> changes one would like to depend on with further changes (I know this well
> :) ), still this seems a flawed argument at least for
> part-time-contributors based KDE projects, where the people one co-operates
> with only have time now and then, like once per week. It could be seen
> unfair & ignorant to them if one simply ignores their opinion, because one
> has more time reserved/ available.

KPIM doesn't have a big active team...

> Not sure where this is from, but often I have seen an unwritten policy
> applied where people for a patch uploaded for review after one week of no
> response add a ping and then wait another week, before finally pushing the
> change. To me this seems a fair and reasonable policy, only ignores people
> who are on vacation for some more weeks or otherwise inactive, but I have
> not seen that ever been a real issue.

2 weeks for a commit, for me it's too long. 
I understand why people can be demotivated when they must wait long time until 
having an answer.
I know that I don't participate a lot on qtcreator dev as we need to wait long 
time to have some review...


> 
> Given the actual purpose of this thread, I would be more curious how you
> have CI integrated in your workflow?

CI: sometime I look at it, sometime not, sometime some guys informs me that 
it's broken (I remember that Luca told me some days ago that a package didn't 
compile, so I fixed it).
Sometime my code compiles on local so for me it's ok but it's just that I 
forgot to trash my builddir.

> And where things could be improved, to
> prevent the current state of unhappiness for people who care about CI some
> more? Given you said you work daily on KDE projects, it seems that the
> brokeness of those projects on the KDE CI has slipped your attention. So
> how does this happen, and how could this be prevented, other than people
> having to hunt you down on irc and tell you :)

I think that Luca idea to send an email directly to the people which breaks 
code when it breaks from several commit is a good idea.

If I received directly a message about kcontact 19.04 after 1 days I fixed it 
directly. Master build correctly and unfortunately I don't have 19.04 and 
master in parallel.

Regards 

> 
> Cheers
> Friedrich


-- 
Laurent Montel | laurent.mon...@kdab.com | KDE/Qt Senior Software Engineer 
KDAB (France) S.A.S., a KDAB Group company Tel. France +33 (0)4 90 84 08 53, 
 www.kdab.fr KDAB - The Qt, C++ and OpenGL Experts - Platform-independent 
software solutions 

Re: CI system maintainability

[Daniel Vrátil]

On Thursday, March 28, 2019 3:39:54 PM CET Friedrich W. H. Kossebau wrote:
> Am Donnerstag, 28. März 2019, 11:27:44 CET schrieb Daniel Vrátil:
> > I'm completely fine with mandatory code review for everything and I'd be
> > happy to have this in PIM. I think the biggest problem in PIM to overcome
> > will be finding the reviewers - I dare say I'm currently the only one who
> > has at least a little idea about what's going on in Akonadi, so getting
> > for
> > instance Laurent to review my Akonadi patches might be hard - same for me
> > reviewing Laurent's KMail patches. This will require non-trivial amount of
> > effort for all members of the community to gain deeper understanding of
> > the
> > other components within the project and a willingness to step up and do a
> > code review even if they don't feel 100% comfortable with the code base.
> > But I believe that in the long run the benefits clearly out-weight the
> > cost.
> So why do you not do this already? Why would you only do this is there is a
> policy requiring you to do it?
> And why do you think this policy-driven behavioural change will work or is
> needed with everyone else?

I wrote this with my PIM hat on and targeted the PIM community, without 
realizing this is a cross-post, so I apologize for the confusion. I believe 
each project should choose whatever workflow and policy suits them.

That said, I do put up for review everything that is not Akonadi/PIM core 
(even changes to PIM components that are not "mine"). The reason I don't do 
this for Akonadi is that there's no-one really to review my code, because no-
one else works on it, or at least that has been my perception of the situation 
lately.

I've been thinking about bringing this topic up on the upcoming PIM sprint as 
I would like to have my code reviewed, I think it's a very good and healthy 
thing, but in case of KDE PIM, I think we need to agree that we'll actually 
review each others code, even if it's not "our own" code-base.

> Also, how do you ensure the review is actually of quality?

How do you ensure that the code people commit without a review is of quality?

> And not just socially driven "+1 for my buddie!", where buddy then also
> mentally shifts part of responsibility to other buddy, because, he, more
> eyes means I do not need to do the full work myself, glitches will be
> caught be them surely! Reviewer found a code formatting issue, done here,
> review happened.

This is a fair point, and I certainly am guilty of doing this occasionally in 
the past. But even reviewer can have a reviewer: if you keep accepting patches 
of "dubious quality" (break build, don't work, contain typos ...), someone 
else from the community will eventually notice and point out you should maybe 
be more careful with your reviews or pay attention to certain aspects (like 
"does it actually work?).

> The opposite also has been seen, reviewers feel they need to do "real"
> review and find things, so start to nitpick or to demand wrong changes,
> wasting everyone's time.
> 
> For myself I know I will happily have other people review my patches, but
> only if there are qualified people to be expected to do it with the needed
> quality in a reasonable time.
> 
> Then, trying to force by that policy other people to become proper
> specialist for certain other code projects to do qualified reviews,
> actually is a trade- off. They will have less time for their actual code
> project, becoming less a specialist there (or having time to review other
> people's contribution to that project).
> We need more contributors, not force existing contributors to distribute
> their abilities & resources over more projects (and thus having less for
> their actual one).

AFAIU the policy right now is keep putting up patches for review until you get 
commit access, then continue doing so until you feel like you don't have to 
anymore. 

But having experienced contributors putting up their code for review is a good 
thing as it allows newcomers (and not just them!) to better understand what is 
happening in the project and simple reviews can be a good starting point for 
them to get more involved in the community.

> I am also not aware of many contributors to KDE projects who are not capable
> to make a responsible decision between the few obvious simple fixes and
> those normal changes which better get feedback from others first. If one is
> unsure about one's post-beer late-night quick hack, one will upload it for
> review, no? At least if one's previous late-night commits turned out to be
> late-night mental state impacted.

This I think is somewhat related to the tooling as I said in my previous 
email. If your workflow for simple obvious fix would be "git push HEAD:refs/
for/master" instead of "git push master", you wouldn't care. If that means 
using arc, I can understand

> To deal with those few which seem challenged to do that decision properly, I
> find such a policy for everyone harmful, I 

Re: CI system maintainability

[Kevin Ottens]

Hello,

On Thursday, 28 March 2019 16:11:12 CET Friedrich W. H. Kossebau wrote:
> Am Donnerstag, 28. März 2019, 14:33:59 CET schrieb laurent Montel:
> > For example I works all days on kde (pim or other) when I wake up, or at
> > noon after my lunch or the evening, I will not wait several days for a
> > review because nobody has time to do it.
> > 
> > (For example I make ~ 15 commits by days on pim/ruqola/framework, I don't
> > want to wait several days/weeks until someone wants to review my patchs)
> 
> Something might be lost in translation here, do you think, because you work
> daily on code of KDE projects, and other people (so potential reviewers) do
> not, this is an argument to do instant pushes of unreviewed commits?
> 
> While I understand one can get impatient if not getting instant review of
> changes one would like to depend on with further changes (I know this well
> :) ),

That particular point is in part a tooling problem though. Phab doesn't make 
this situation easy to handle. I have to do very naughty things to git and arc 
to deal with those. I'm guilty of often piling a dozen patches and rewriting 
extensively my history locally before the lots hits the first round of 
reviews. :-)

> still this seems a flawed argument at least for part-time-contributors based
> KDE projects, where the people one co-operates with only have time now and
> then, like once per week. It could be seen unfair & ignorant to them if one
> simply ignores their opinion, because one has more time reserved/ available.

This is one of the big flaws of self-proclaimed meritocratic communities. You 
can out-commit someone and end up being the big decision maker hard to 
convince. Doesn't happen by malice in most cases I think, but it's a clear 
slippery slope.

> Not sure where this is from, but often I have seen an unwritten policy
> applied where people for a patch uploaded for review after one week of no
> response add a ping and then wait another week, before finally pushing the
> change. To me this seems a fair and reasonable policy, only ignores people
> who are on vacation for some more weeks or otherwise inactive, but I have
> not seen that ever been a real issue.

Agreed. It makes sense.
 
> Given the actual purpose of this thread, I would be more curious how you
> have CI integrated in your workflow? And where things could be improved, to
> prevent the current state of unhappiness for people who care about CI some
> more? Given you said you work daily on KDE projects, it seems that the
> brokeness of those projects on the KDE CI has slipped your attention. So
> how does this happen, and how could this be prevented, other than people
> having to hunt you down on irc and tell you :)

Definitely interested in this as well. It's not the first time we have Ben 
having to escalate to some form of threat regarding the state of the CI in PIM 
components. Although it's clear from the kde-pim list that the CI is making a 
lot of noise there. Either we collectively became too good at ignoring those 
emails, or it's too verbose (after all it's one email per component per build 
type, it piles up quickly).

Regards.
-- 
Kevin Ottens, http://ervin.ipsquad.net


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

[Friedrich W. H. Kossebau]

Hi Laurent,

Am Donnerstag, 28. März 2019, 14:33:59 CET schrieb laurent Montel:
> For example I works all days on kde (pim or other) when I wake up, or at
> noon after my lunch or the evening, I will not wait several days for a
> review because nobody has time to do it.
> 
> (For example I make ~ 15 commits by days on pim/ruqola/framework, I don't
> want to wait several days/weeks until someone wants to review my patchs)

Something might be lost in translation here, do you think, because you work 
daily on code of KDE projects, and other people (so potential reviewers) do 
not, this is an argument to do instant pushes of unreviewed commits?

While I understand one can get impatient if not getting instant review of 
changes one would like to depend on with further changes (I know this well :) 
), still this seems a flawed argument at least for part-time-contributors 
based KDE projects, where the people one co-operates with only have time now 
and then, like once per week. It could be seen unfair & ignorant to them if 
one simply ignores their opinion, because one has more time reserved/
available.

Not sure where this is from, but often I have seen an unwritten policy 
applied where people for a patch uploaded for review after one week of no 
response add a ping and then wait another week, before finally pushing the 
change. To me this seems a fair and reasonable policy, only ignores people 
who are on vacation for some more weeks or otherwise inactive, but I have not 
seen that ever been a real issue.

Given the actual purpose of this thread, I would be more curious how you have 
CI integrated in your workflow? And where things could be improved, to 
prevent the current state of unhappiness for people who care about CI some 
more? Given you said you work daily on KDE projects, it seems that the 
brokeness of those projects on the KDE CI has slipped your attention.
So how does this happen, and how could this be prevented, other than people 
having to hunt you down on irc and tell you :)

Cheers
Friedrich

D20086: Fix window height of Screen Locking KCM

[Andres Betts]

abetts added a comment.


  +1

REPOSITORY
  R133 KScreenLocker

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

To: tigrang, davidedmundson
Cc: abetts, filipf, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

Re: CI system maintainability

[Boudhayan Gupta]

Hi,

On Thu, 28 Mar 2019 at 15:21, Kevin Ottens  wrote:

> Hello,
>
> On Thursday, 28 March 2019 14:33:59 CET laurent Montel wrote:
> > I am against to force mandatory review, as it will create a lot of lose
> of
> > time,
>
> As I said, unpopular.
>

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

I don't care if you lose time. I don't want the guys building my house to
cut corners mixing my concrete because it's going to save time. Why are you
in such a massive hurry to make changes to software which for example holds
access to my Google Account password? In fact, the very fact that you make
this argument makes me wonder if I'm running trustable code on my computer
at all, because apparently doing it quickly is far more important than
doing it right.

As a user, I simply do not want unreviewed crap running on my computer.
Yes, crap, because no software engineer writes bug-free code all the time,
and if you're so overconfident that you don't need reviews on even your
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.

As a developer, I know that even one-liners, and especially one-liners, the
sort where you think "meh, this is a tiny little thing, I don't have to be
careful" are the ones that have the most dangerous typos and unintended
bugs. Reviews catch that.

In a project like PIM, if the code hasn't been through review, which
independent party do I trust to verify that you're not, for example,
leaking my Google password to some world-readable tempfile? Do you really
expect every user to read the entire codebase for themselves and make sure
that's not being done? The whole point of having all the code out in the
open for independent audit purposes, to protect your security and privacy
and what not is completely moot if no one else actually looks at the code
anyway. And let's be honest, the code quality of some of KDE's projects - I
wouldn't touch them with a six-foot pole. The ones I would touch though,
all have multiple people looking at the code and reviewing everything that
goes in.

Let me be very clear - even if you're the best damn programmer on the
planet, if *you* wrote the code, I do not trust *you* one inch to tell me
that that code is correct. That verification needs to come from someone
else, someone who does not have a conflict of interest in seeing that code
get into production. This is nothing personal, this is confirmation bias on
the author's part which leads to issues that even though they might be
infrequent, usually have catastrophic impact.

And if "culture" trumps over engineering best practices, it follows that I
should just stop using software produced by this entity because who knows
what it's doing.

Thanks,
Boudhayan

Re: CI system maintainability

[David Jarvie]

On 28 March 2019 13:33:59 GMT, laurent Montel  wrote:
> Le jeudi 28 mars 2019, 09:29:22 CET Kevin Ottens a écrit :
> > Hello,
> > 
> > On Thursday, 28 March 2019 09:16:11 CET Ben Cooksley wrote:
> > > Please note that the commits in this instance were pushed without
> > > review, so restrictions on merge requests wouldn't make a
> difference
> > > in this case unfortunately.
> > 
> > Maybe it's about time to make reviews mandatory... I know it's
> unpopular in
> > KDE, and I advocated for "don't force a tool if you can get someone
> to look
> > 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.
> > 
> > Regards.
> 
> Hi,
> 
> I am against to force mandatory review, as it will create a lot of
> lose of 
> time, and we will not be sure that review is correct (see comment from
> Volker 
> about "transaction lock regression")
> 
> It's necessary to having a big team for doing it.
> 
> Ok a repo was broken, but it was just that fix was created in master
> not 
> 19.04, I didn't see nobody on IRC told us "this package is always
> broken", 
> when I saw it this morning I just cherry pick (2 seconds for fixing
> it).
> 
> 
> For example I works all days on kde (pim or other) when I wake up, or
> at noon 
> after my lunch or the evening, I will not wait several days for a
> review 
> because nobody has time to do it. (we can see a review from zanshin
> for 
> example https://phabricator.kde.org/D16210 we can see that David
> waited 2 
> months until having an answer...).
> 
> (For example I make ~ 15 commits by days on pim/ruqola/framework, I
> don't want 
> to wait several days/weeks until someone wants to review my patchs)
> 
> I will not lose my time to review some code that I don't understand...
> I never 
> reviewed Akonadi patch as I don't understand this code, and I will
> take time 
> on it just for the pleasure as I prefer fixing bug or adding new
> features in 
> components that I maintain.
> 
> When we have a big team as Qt team it can help but in pim component
> where we 
> don't have any redundant guy, we will lose a lot of time.
> 
> So for each increase version for each package I will wait a review.
> For sure 
> not.
> 
> Each time that I will improve code as coding style I will wait that
> someone 
> wants to review...

I agree. Mandatory reviews might work if there is a team of active people 
working on a project, but if there is only one person with real knowledge of 
the code, or there is nobody else with much time to spare, who is going to do 
the review? It is likely to just sit waiting indefinitely. If getting code 
reviewed is too difficult, the developer may have to give up and abandon the 
project.

Mandatory reviewing could only work if individual projects can decide whether 
to adopt it or not.

--
David Jarvie
KAlarm author, KDE developer
http://www.astrojar.org.uk/kalarm

Re: CI system maintainability

[Friedrich W. H. Kossebau]

Am Donnerstag, 28. März 2019, 11:27:44 CET schrieb Daniel Vrátil:
> I'm completely fine with mandatory code review for everything and I'd be
> happy to have this in PIM. I think the biggest problem in PIM to overcome
> will be finding the reviewers - I dare say I'm currently the only one who
> has at least a little idea about what's going on in Akonadi, so getting for
> instance Laurent to review my Akonadi patches might be hard - same for me
> reviewing Laurent's KMail patches. This will require non-trivial amount of
> effort for all members of the community to gain deeper understanding of the
> other components within the project and a willingness to step up and do a
> code review even if they don't feel 100% comfortable with the code base.
> But I believe that in the long run the benefits clearly out-weight the
> cost.

So why do you not do this already? Why would you only do this is there is a 
policy requiring you to do it?
And why do you think this policy-driven behavioural change will work or is 
needed with everyone else?

Also, how do you ensure the review is actually of quality?
And not just socially driven "+1 for my buddie!", where buddy then also 
mentally shifts part of responsibility to other buddy, because, he, more eyes 
means I do not need to do the full work myself, glitches will be caught be 
them surely! Reviewer found a code formatting issue, done here, review 
happened.
The opposite also has been seen, reviewers feel they need to do "real" review 
and find things, so start to nitpick or to demand wrong changes, wasting 
everyone's time.

For myself I know I will happily have other people review my patches, but 
only if there are qualified people to be expected to do it with the needed 
quality in a reasonable time.

Then, trying to force by that policy other people to become proper specialist 
for certain other code projects to do qualified reviews, actually is a trade-
off. They will have less time for their actual code project, becoming less a 
specialist there (or having time to review other people's contribution to 
that project).
We need more contributors, not force existing contributors to distribute 
their abilities & resources over more projects (and thus having less for 
their actual one).

I am also not aware of many contributors to KDE projects who are not capable 
to make a responsible decision between the few obvious simple fixes and those 
normal changes which better get feedback from others first. If one is unsure 
about one's post-beer late-night quick hack, one will upload it for review, 
no? At least if one's previous late-night commits turned out to be late-night 
mental state impacted.
To deal with those few which seem challenged to do that decision properly, I 
find such a policy for everyone harmful, I know it would impact my 
contribution willingness, when I come across a typo or other simple and 
obvious to fix things. And just leave the garbage around along my current 
working path.

Besides, it will not solve the issue this thread is about, with some people 
not caring (quickly) enough if CI builds fail or if there are regressions in 
tests.

Review builds once implemented and deployed will help there partially as a 
side-effect only, catching some build fails before. But also not if this 
breaks (e.g. due to API/behavioural changes) depending projects, as CI at 
least currently does not rebuild dependent projects (cmp. also T7374).

A society with people doing things only due to policies and not intrinsic 
motivation seems very flawed to me, makes me wonder why people are in there 
given no-one forced them into this society.

https://community.kde.org/Policies/
Commit_Policy#Code_review_by_other_developers has some policies already, 
which should be enough:
1.1 Think Twice before Committing
1.2 Never commit code that doesn't compile
1.3 Test your changes before committing
1.4 Double check what you commit
1.10 Code review by other developers
1.11 Take responsibility for your commits
1.12 Don't commit code you don't understand
Well, perhaps could be amended by an explicit note about keeping CI working.

Enforcing review of any commits IMHO should be only done for people who 
notoriously failed to comply with those existing rules. If we are unable to 
pinpoint those people, talk with them and make them comply or sort out their 
reasons to not yet comply, but instead create as workaround new generic 
policies for everyone, we make this a worse society. And also a less 
effective, with more rubber stamps needed.

Cheers
Friedrich

D20093: Show a context menu on pressAndHold

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> Task.qml:134
>  
> +onPressAndHold: {
> +if (model.IsLauncher === true) {

This item also has drag and drop

Will the context menu open mid-way through dragging?

REPOSITORY
  R119 Plasma Desktop

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

To: apol, #plasma, mart
Cc: davidedmundson, hein, plasma-devel, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D20093: Show a context menu on pressAndHold

[Eike Hein]

hein added a comment.


  I guess this brings us back to the old debate of a generic solution vs. 
patching all the UIs. I'm inclined to agree with this approach because it 
should probably be up to the specific UI what it wants to happen on the gesture.
  
  It means we need to be careful how we use it, though. On the desktop we also 
have press and hold to engage applet move mode, which means it's already 
off-limits for applet context menus there. Is this change going to conflict 
with that wrt/ user expectations?

REPOSITORY
  R119 Plasma Desktop

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

To: apol, #plasma, mart
Cc: hein, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

Re: CI system maintainability

[Kevin Ottens]

Hello,

On Thursday, 28 March 2019 14:33:59 CET laurent Montel wrote:
> I am against to force mandatory review, as it will create a lot of lose of
> time,

As I said, unpopular.

> and we will not be sure that review is correct (see comment from
> Volker about "transaction lock regression")

This argument is absurd: "Hey, this vest isn't 100% bullet-proof, thus I'll 
walk in the middle of a war naked".

> It's necessary to having a big team for doing it.

I agree having a team of 1 for each KDE PIM component is a problem (and often 
the same person).

> Ok a repo was broken, but it was just that fix was created in master not
> 19.04, I didn't see nobody on IRC told us "this package is always broken",
> when I saw it this morning I just cherry pick (2 seconds for fixing it).
> 
> For example I works all days on kde (pim or other) when I wake up, or at
> noon after my lunch or the evening, I will not wait several days for a
> review because nobody has time to do it.

> (we can see a review from zanshin for example https://phabricator.kde.org/
> D16210 we can see that David waited 2 months until having an answer...).

Unfair, if you read the comments you'll see that this particular patch didn't 
appear in my dashboard for some odd reason (I suspect it comes from how arc 
associates a patch to a repository or not, but I digress...).

> (For example I make ~ 15 commits by days on pim/ruqola/framework, I don't
> want to wait several days/weeks until someone wants to review my patchs)
> 
> I will not lose my time to review some code that I don't understand... I
> never reviewed Akonadi patch as I don't understand this code, and I will
> take time on it just for the pleasure as I prefer fixing bug or adding new
> features in components that I maintain.
> 
> When we have a big team as Qt team it can help but in pim component where we
> don't have any redundant guy, we will lose a lot of time.

Ah, the mythical big team... Because it's well known that on Qt the reviewers 
aren't stretch thin and that patches never end up weeks in limbo...

Also you know that if you don't change your way of working you'll stay alone 
on your components? I know, chicken and egg... but all those commits and stuff 
randomly changing all the time doesn't help people to get in even if they'd 
want to.

But you know that very well, we just discussed it half a dozen times in the 
past...

> So for each increase version for each package I will wait a review. For sure
> not.
> 
> Each time that I will improve code as coding style I will wait that someone
> wants to review...
> 
> 
> I know that it's easy to write "make reviews mandatory" there is not an
> impact about your work as we know that you are not really active on kde at
> the moment...

Right, let's bring something irrelevant to the discussion. You know it's 
mainly due to health issues the past few months right? (yes, March has been a 
real joy again... it keeps on giving)

Besides, I apply mandatory reviews to myself on zanshin.

> For sure I broke kcontact 2 days ago, but as a friend told me when I started
> to work on kde "We can't create a bug when we don't code..."

And this is true, writing code will create bugs, that's why responsible people 
like safety nets even to the price of throughput.

Now can we use a more adult tone again? Maybe re-read Dan's email again who 
has a more balanced view despite being in a similar situation?

Regards.
-- 
Kevin Ottens, http://ervin.ipsquad.net


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

Re: CI system maintainability

[Nate Graham]

With regards to the discussion about mandatory code review, I think it's 
important to avoid immediately rushing to create new policy as a result of a 
particular event or abuse. It's always tempting to try to put in place a rule 
that would have avoided the problem if it had existed and was being followed, 
but usually in these circumstances, existing rules or conventions were already 
being violated. So adding new ones usually doesn't help as much as people would 
want because they don't address the underlying issue of why rules are being 
broken in the first place.

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.

Nate

D20093: Show a context menu on pressAndHold

[Aleix Pol Gonzalez]

apol created this revision.
apol added reviewers: Plasma, mart.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
apol requested review of this revision.

REVISION SUMMARY
  Depends on D20085 

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

AFFECTED FILES
  applets/kicker/package/contents/ui/CompactRepresentation.qml
  applets/kickoff/package/contents/ui/Kickoff.qml
  applets/taskmanager/package/contents/ui/Task.qml
  desktoppackage/contents/applet/DefaultCompactRepresentation.qml
  desktoppackage/contents/views/Desktop.qml
  desktoppackage/contents/views/Panel.qml

To: apol, #plasma, mart
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19822: [Task Manager] Toggle mute when the audio indicator is clicked

[Eike Hein]

hein added a subscriber: mart.
hein added a comment.


  rooty, your behavior in this thread has been extremely aggressive, 
disrespectful and uncollaborative. I'd like to ask you to stop at this point 
and not participate futher, as well as in any future threads related to the 
Task Manager applets.
  
  Back to work: For this patch to proceed further, please remove the setting.
  
  For the hover feedback, I'd like to get @mart's opinion on Kirigami.Icon vs. 
whether we should look into making PlasmaComponents fitter in this regard.

REPOSITORY
  R119 Plasma Desktop

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

To: faridb, #plasma, #vdg, ngraham, filipf, hein
Cc: mart, rooty, hein, filipf, mvourlakos, ngraham, broulik, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D19822: [Task Manager] Toggle mute when the audio indicator is clicked

[Nathaniel Graham]

ngraham added a comment.


  Hey guys,
  I propose that we shelve the discussion about making this optional since it 
seems like we all agree that it should be on by default. In my experience, 
focusing on the points of //dis//agreement makes it likely that nothing gets 
landed, and relationships get damaged in the process. Once we've got the 
feature polished up, endowed with a solid user interface, and in production, 
maybe at that point we can evaluate from a position of having more information 
whether we really need an option for it.
  
  Thanks!

REPOSITORY
  R119 Plasma Desktop

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

To: faridb, #plasma, #vdg, ngraham, filipf, hein
Cc: rooty, hein, filipf, mvourlakos, ngraham, broulik, plasma-devel, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

Re: CI system maintainability

[laurent Montel]

Le jeudi 28 mars 2019, 09:29:22 CET Kevin Ottens a écrit :
> Hello,
> 
> On Thursday, 28 March 2019 09:16:11 CET Ben Cooksley wrote:
> > Please note that the commits in this instance were pushed without
> > review, so restrictions on merge requests wouldn't make a difference
> > in this case unfortunately.
> 
> Maybe it's about time to make reviews mandatory... I know it's unpopular in
> KDE, and I advocated for "don't force a tool if you can get someone to look
> 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.
> 
> Regards.

Hi,

I am against to force mandatory review, as it will create a lot of lose of 
time, and we will not be sure that review is correct (see comment from Volker 
about "transaction lock regression")

It's necessary to having a big team for doing it.

Ok a repo was broken, but it was just that fix was created in master not 
19.04, I didn't see nobody on IRC told us "this package is always broken", 
when I saw it this morning I just cherry pick (2 seconds for fixing it).


For example I works all days on kde (pim or other) when I wake up, or at noon 
after my lunch or the evening, I will not wait several days for a review 
because nobody has time to do it. (we can see a review from zanshin for 
example https://phabricator.kde.org/D16210 we can see that David waited 2 
months until having an answer...).

(For example I make ~ 15 commits by days on pim/ruqola/framework, I don't want 
to wait several days/weeks until someone wants to review my patchs)

I will not lose my time to review some code that I don't understand... I never 
reviewed Akonadi patch as I don't understand this code, and I will take time 
on it just for the pleasure as I prefer fixing bug or adding new features in 
components that I maintain.

When we have a big team as Qt team it can help but in pim component where we 
don't have any redundant guy, we will lose a lot of time.

So for each increase version for each package I will wait a review. For sure 
not.

Each time that I will improve code as coding style I will wait that someone 
wants to review...


I know that it's easy to write "make reviews mandatory" there is not an impact 
about your work as we know that you are not really active on kde at the 
moment...

For sure I broke kcontact 2 days ago, but as a friend told me when I started 
to work on kde "We can't create a bug when we don't code..."

Regards

-- 
Laurent Montel | laurent.mon...@kdab.com | KDE/Qt Senior Software Engineer 
KDAB (France) S.A.S., a KDAB Group company Tel. France +33 (0)4 90 84 08 53, 
 www.kdab.fr KDAB - The Qt, C++ and OpenGL Experts - Platform-independent 
software solutions 

D19822: [Task Manager] Toggle mute when the audio indicator is clicked

[Krešimir Čohar]

rooty added a comment.


  > In projects where maintenance is lacking you will notice a trend towards 
options sprawl for that reason.
  
  That doesn't make sense. It might mean there's a lack of direction but not 
necessarily a lack of maintenance. Whether expansion leads to "option sprawl" 
really is a matter of opinion.
  
  > The suggestion wrt/ hover feedback that you all seem to like comes from 
asking "what's the actual problem, and is an option the best way to address 
it?". See why the mental habit is useful?
  
  That is a leading question (and a strawman fallacy) based on the supposition 
that there is a problem. It may be and most often is a matter of preference.
  
  > "Plasma does the right thing without needing to be tweaked in the first 
place" is a more lofty and mature aspiration than "Plasma is infinitely 
tweakable".
  
  Plasma being infinitely tweakable doesn't entail its not doing "the right 
thing" in the first place. Versatility doesn't mean indecisiveness.
  
  > "We had so many options we went from one page to two pages, so now we can 
add more options!" :-)
  
  Just use the one page then and claim there's no more room for more features 
:-)

REPOSITORY
  R119 Plasma Desktop

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

To: faridb, #plasma, #vdg, ngraham, filipf, hein
Cc: rooty, hein, filipf, mvourlakos, ngraham, broulik, plasma-devel, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

Re: CI system maintainability

[Kevin Ottens]

Hello,

On Thursday, 28 March 2019 11:27:44 CET Daniel Vrátil wrote:
> I'm completely fine with mandatory code review for everything and I'd be
> happy to have this in PIM. I think the biggest problem in PIM to overcome
> will be finding the reviewers - I dare say I'm currently the only one who
> has at least a little idea about what's going on in Akonadi, so getting for
> instance Laurent to review my Akonadi patches might be hard - same for me
> reviewing Laurent's KMail patches. This will require non-trivial amount of
> effort for all members of the community to gain deeper understanding of the
> other components within the project and a willingness to step up and do a
> code review even if they don't feel 100% comfortable with the code base.
> But I believe that in the long run the benefits clearly out-weight the
> cost.

This! :-)
 
> Btw we practice this policy at work and I do truly appreciate it, not only
> as a huge learning experience but so many times just having a second pair
> of eyes to glance over my code has revealed issues that sometimes almost
> make me question my career choice as a programmer :-) I think this is
> especially important for projects like PIM, where most of us contribute at
> work in between waiting for CI and replying to 15 Slack threads or in the
> evening after a long day

And this too of course.

I fully support this message. ;-)

Cheers.
-- 
Kevin Ottens, http://ervin.ipsquad.net


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

KDE CI: Plasma » plasma-workspace » kf5-qt5 SUSEQt5.12 - Build # 107 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Plasma/job/plasma-workspace/job/kf5-qt5%20SUSEQt5.12/107/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Tue, 26 Mar 2019 18:57:43 +
 Build duration:
30 min and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/LibColorCorrect-5.15.80.xmlacc/LibKWorkspace-5.15.80.xmlacc/LibTaskManager-5.15.80.xmlcompat_reports/LibColorCorrect_compat_report.htmlcompat_reports/LibKWorkspace_compat_report.htmlcompat_reports/LibTaskManager_compat_report.htmllogs/LibColorCorrect/5.15.80/log.txtlogs/LibKWorkspace/5.15.80/log.txtlogs/LibTaskManager/5.15.80/log.txt
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.kioslave.desktop Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.klipper Failed: 0 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 2 test(s)Name: projectroot.libcolorcorrect Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.libkworkspace Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.libtaskmanager Failed: 0 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 2 test(s)Name: projectroot.runners.bookmarks Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.runners.services Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.shell Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report15%
(18/122)12%
(46/381)12%
(46/381)7%
(2469/34715)5%
(1262/24146)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsapplets.appmenu.lib0%
(0/2)0%
(0/2)0%
(0/128)0%
(0/84)applets.appmenu.plugin0%
(0/3)0%
(0/3)0%
(0/188)0%
(0/150)applets.calendar0%
(0/1)0%
(0/1)0%
(0/6)100%
(0/0)applets.digital-clock.plugin0%
(0/5)0%
(0/5)0%
(0/943)0%
(0/92)applets.icon0%
(0/1)0%
(0/1)0%
(0/279)0%
(0/186)applets.notifications.lib0%
(0/1)0%
(0/1)0%
(0/52)0%
(0/26)applets.notifications.plugin0%
(0/5)0%
(0/5)0%
(0/300)0%
(0/130)applets.systemtray0%
(0/1)0%
(0/1)0%
(0/269)0%
(0/275)applets.systemtray.container0%
(0/1)0%
(0/1)0%
(0/60)0%
(0/52)applets.systemtray.tests.statusnotifier0%
(0/3)0%
(0/3)0%
(0/182)0%
(0/40)appmenu0%
(0/7)0%
(0/7)0%
(0/170)0%
(0/66)components.keyboardlayout0%
(0/2)0%
(0/2)0%
(0/76)0%
(0/32)components.sessionsprivate0%
(0/3)0%
(0/3)0%
  

Re: CI system maintainability

[Michael Reeves]

On Thu, Mar 28, 2019, 6:36 AM Friedrich W. H. Kossebau 
wrote:

> Am Donnerstag, 28. März 2019, 09:29:22 CET schrieb Kevin Ottens:
> > Hello,
> >
> > On Thursday, 28 March 2019 09:16:11 CET Ben Cooksley wrote:
> > > Please note that the commits in this instance were pushed without
> > > review, so restrictions on merge requests wouldn't make a difference
> > > in this case unfortunately.
> >
> > Maybe it's about time to make reviews mandatory... I know it's unpopular
> in
> > KDE, and I advocated for "don't force a tool if you can get someone to
> look
> > 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.
>

Then fix what's broken. If these projects need manditory reviews fine but
don't take a one-size-fits-all approach.

>
>
> Mandatory reviews in my experience only result in more fake reviews due to
> people pressuring each other to quickly get their simple patches reviewed,
> lowering the general quality of reviews.
> Also does the overhead reduce the number of minor improvements, where one
> (as
> qualified person) simply would have pushed in a minute a fix and get back
> to
> concentrate on the real work, instead of starting an overhead of having to
> juggle with yet another patch-under-review where the current work depends
> on.
>
> IMHO the actual problem here is: people do not do their post-push work and
> care for the state on CI.
>

Agreed.

>
> From what I saw, many breakages happened with reviewed patches. Whole
> releases even get done while CI is reporting failed builds, or at least
> lots
> of failing tests.
>

Requiring pre-commit hooks which run these could be helpful. They could
stop this at the local machine. Perhaps also a reminder to check ci. Not
sure this completely solves the issue but it would be workable for small
projects like kdiff3 and would reduce overhead for minor typo correction.

>
> I have no idea how to fix that. We would need to ask the people for whom
> this
> happens why it does happen, and how we can improve things so that CI
> checks
> become part of their workflow.
>
> Cheers
> Friedrich
>
>
>

Re: CI system maintainability

[Johannes Zarl-Zierl]

Hi,

(Sorry for top-posting)

I fear that a mandatory reviews would add too juch strain on smaller teams. If 
there's just one person with an intimate knowledge of the code-base, plus two 
co-developers, then who should do the reviews?

How about a distinction based on importance of a project instead? I.e. 
mandatory reviews for frameworks and any app that wants to be included in KDE 
apps releases. Non-mandatory reviews can then also come with a "price" to pay: 
if CI errors are not dealt with in a timely manner, you should be free to 
disable the CI for administrative reasons...

  Johannes

Am 28. März 2019 10:17:18 MEZ schrieb Tomaz Canabrava :
>On Thu, Mar 28, 2019 at 10:09 AM Luca Beltrame 
>wrote:
>>
>> 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.
>
>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.
>I strongly advocate for "reviews always", but since I was outvoted a
>few times on this I would at least say "can we at least have reviews
>for non project members" ?
>
>
>> --
>> Luca Beltrame
>> GPG key ID: A29D259B

Re: CI system maintainability

[Konstantin Kharlamov]

On Чт, Mar 28, 2019 at 19:40, Ben Cooksley  wrote:

Hi all,

We currently have a rather substantial issue, in that the CI system
has been once again left in a position where it isn't possible to make
any changes to the system.

This means we can't update to newer versions of packages, add new
packages or correct for binary incompatible changes which periodically
get introduced to non-Frameworks.

This issue has arisen because currently we have a recurring failure to
build from source, within KDE PIM. Specifically, KContacts fails due
to broken CMake logic. Despite this breakage having been in place for
several days now, and the relevant mailing list being informed
automatically by the CI system, the issue has not been corrected.

While the most immediate fix is to correct this failure to build from
source, that is only a short term fix and does not fix the underlying
issue which makes the CI system difficult to maintain - and that is
build failure reports being ignored, and people pushing broken code
that doesn't even build.

(For those wondering, the CI system uses OpenSUSE Tumbleweed, a
rolling release distribution, for it's builds, so it isn't a case of
old CMake or anything along those lines)

We therefore need a long term fix for this. Note that pre-commit (as
part of review) CI is not a solution in this instance, as the
offending commits did not go through review.

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

At this point given the amount of effort required to maintain a CI
system vs. the amount of care actually being given by some developers
(who are ignoring it's failure emails) it becomes questionable whether
the effort is worth the return (and if not, we should just shut it
down)

Regards,
Ben Cooksley
KDE Sysadmin


I don't know abut the current CI, but judging by recent discussion that 
is about KDE migrating to gitlab; quick search shows gitlab does allow 
prohibiting a merge if CI failed¹


Note however, in my experience of contributing to diffrent project CI 
often fails for reasons absolutely irrelevant to code being tested 
(e.g. errors in a CI script), in this case prohibiting a merge may 
worsen the situation.


1: 
https://docs.gitlab.com/ee/user/project/merge_requests/merge_when_pipeline_succeeds.html#only-allow-merge-requests-to-be-merged-if-the-pipeline-succeeds

KDE CI: Plasma » plasma-workspace » stable-kf5-qt5 SUSEQt5.12 - Build # 51 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Plasma/job/plasma-workspace/job/stable-kf5-qt5%20SUSEQt5.12/51/
 Project:
stable-kf5-qt5 SUSEQt5.12
 Date of build:
Tue, 26 Mar 2019 18:57:43 +
 Build duration:
15 min and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/LibColorCorrect-5.15.3.xmlacc/LibKWorkspace-5.15.3.xmlacc/LibTaskManager-5.15.3.xmlcompat_reports/LibColorCorrect_compat_report.htmlcompat_reports/LibKWorkspace_compat_report.htmlcompat_reports/LibTaskManager_compat_report.htmllogs/LibColorCorrect/5.15.3/log.txtlogs/LibKWorkspace/5.15.3/log.txtlogs/LibTaskManager/5.15.3/log.txt
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.kioslave.desktop Failed: 1 test(s), Passed: 0 test(s), Skipped: 0 test(s), Total: 1 test(s)Failed: projectroot.kioslave.desktop.tests.testdesktopName: projectroot.klipper Failed: 0 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 2 test(s)Name: projectroot.libcolorcorrect Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.libkworkspace Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.libtaskmanager Failed: 0 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 2 test(s)Name: projectroot.runners.bookmarks Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.runners.services Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.shell Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report15%
(18/122)12%
(47/380)12%
(47/380)7%
(2473/34659)5%
(1265/24106)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsapplets.appmenu.lib0%
(0/2)0%
(0/2)0%
(0/128)0%
(0/84)applets.appmenu.plugin0%
(0/3)0%
(0/3)0%
(0/188)0%
(0/150)applets.calendar0%
(0/1)0%
(0/1)0%
(0/6)100%
(0/0)applets.digital-clock.plugin0%
(0/5)0%
(0/5)0%
(0/943)0%
(0/92)applets.icon0%
(0/1)0%
(0/1)0%
(0/263)0%
(0/180)applets.notifications.lib0%
(0/1)0%
(0/1)0%
(0/52)0%
(0/26)applets.notifications.plugin0%
(0/5)0%
(0/5)0%
(0/300)0%
(0/130)applets.systemtray0%
(0/1)0%
(0/1)0%
(0/269)0%
(0/275)applets.systemtray.container0%
(0/1)0%
(0/1)0%
(0/60)0%
(0/52)applets.systemtray.tests.statusnotifier0%
(0/3)0%
(0/3)0%
(0/182)0%
(0/40)appmenu0%
(0/7)0%
(0/7)0%
(0/170)0%
(0/66)components.keyboardlayout0%
(0/2)0%
(0/2)0%
(0/76)0%
(0/32)components.sessionsprivate0%
(0/3)0%

D20086: Fix window height of Screen Locking KCM

[Filip Fila]

filipf added a comment.


  Good fix, the lack of implicitHeight was leading to confusion before if there 
are even any wallpapers present.
  
  One question though: wouldn't it be better to define the height in 
`units.gridUnit` instead of raw pixels, or even absolutely by doubling the 
height of one grid item? From my understanding, when scaling is changed the 
grid items are also resized so the window won't open showing 2 rows of 
wallpapers anymore, as was desired.

REPOSITORY
  R133 KScreenLocker

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

To: tigrang, davidedmundson
Cc: filipf, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19971: Fix compilation error when SA_TRACE is 1

[Vlad Zagorodniy]

This revision was automatically updated to reflect the committed changes.
Closed by commit R111:55aa93600b61: Fix compilation error when SA_TRACE is 1 
(authored by zzag).

REPOSITORY
  R111 KSysguard Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19971?vs=54550=54970

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

AFFECTED FILES
  ksgrd/SensorAgent.cpp

To: zzag, davidedmundson
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D12055: Remove border around menubars

[Vlad Zagorodniy]

This revision was automatically updated to reflect the committed changes.
Closed by commit R98:1c66db4f34c9: Remove border around menubars (authored by 
zzag).

REPOSITORY
  R98 Breeze for Gtk

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12055?vs=54576=54969

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

AFFECTED FILES
  src/gtk318/widgets/_menus.scss
  src/gtk320/widgets/_menus.scss

To: zzag, #breeze, ngraham, jackg, #vdg
Cc: abetts, ndavis, filipf, jackg, ngraham, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, 
mart

Re: CI system maintainability

[Friedrich W. H. Kossebau]

Am Donnerstag, 28. März 2019, 09:29:22 CET schrieb Kevin Ottens:
> Hello,
> 
> On Thursday, 28 March 2019 09:16:11 CET Ben Cooksley wrote:
> > Please note that the commits in this instance were pushed without
> > review, so restrictions on merge requests wouldn't make a difference
> > in this case unfortunately.
> 
> Maybe it's about time to make reviews mandatory... I know it's unpopular in
> KDE, and I advocated for "don't force a tool if you can get someone to look
> 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.

Mandatory reviews in my experience only result in more fake reviews due to 
people pressuring each other to quickly get their simple patches reviewed, 
lowering the general quality of reviews.
Also does the overhead reduce the number of minor improvements, where one (as 
qualified person) simply would have pushed in a minute a fix and get back to 
concentrate on the real work, instead of starting an overhead of having to 
juggle with yet another patch-under-review where the current work depends on.

IMHO the actual problem here is: people do not do their post-push work and 
care for the state on CI.

From what I saw, many breakages happened with reviewed patches. Whole 
releases even get done while CI is reporting failed builds, or at least lots 
of failing tests.

I have no idea how to fix that. We would need to ask the people for whom this 
happens why it does happen, and how we can improve things so that CI checks 
become part of their workflow.

Cheers
Friedrich

Re: CI system maintainability

[Daniel Vrátil]

On Thursday, March 28, 2019 9:50:47 AM CET Kevin Ottens wrote:
> Hello,
> 
> On Thursday, 28 March 2019 09:41:29 CET Luca Beltrame wrote:
> > 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.

This is partially a problem of tooling IMO - review for even a trivial change 
will not cause any friction, if the tooling makes it super-easy and natural 
part of your workflow (and then you can always poke someone on IRC "hey, do 
you have 5 seconds for this super-simple review?").

Using arc with Phab is a bit annoying, so I can understand people fighting 
this. Gitlab with their "click this link to turn your commit into a merge 
request" is much better, plus it can be merged by the reviewer with a single 
click so you as a developer don't even need to care about the MR anymore.

> I don't fully disagree with this (although I'd have stories on nefarious
> typos even for what was supposed to be a "trivial fix"). But it becomes a
> question of trade-off, IOW "how hurtful to the project mandatory reviews?"
> vs "how hurtful to the project a central component being very regularly
> broken?".
> 
> I'd argue we're loosing more with the current state of PIM than we'd loose
> with mandatory reviews.

I'm completely fine with mandatory code review for everything and I'd be happy 
to have this in PIM. I think the biggest problem in PIM to overcome will be 
finding the reviewers - I dare say I'm currently the only one who has at least 
a little idea about what's going on in Akonadi, so getting for instance 
Laurent to review my Akonadi patches might be hard - same for me reviewing 
Laurent's KMail patches. This will require non-trivial amount of effort for 
all members of the community to gain deeper understanding of the other 
components within the project and a willingness to step up and do a code 
review even if they don't feel 100% comfortable with the code base. But I 
believe that in the long run the benefits clearly out-weight the cost.

Btw we practice this policy at work and I do truly appreciate it, not only as 
a huge learning experience but so many times just having a second pair of eyes 
to glance over my code has revealed issues that sometimes almost make me 
question my career choice as a programmer :-) I think this is especially 
important for projects like PIM, where most of us contribute at work in 
between waiting for CI and replying to 15 Slack threads or in the evening 
after a long day

Cao,
Dan

> Regards.


-- 
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: CI system maintainability

[Volker Krause]

On Thursday, 28 March 2019 09:50:47 CET Kevin Ottens wrote:
> Hello,
> 
> On Thursday, 28 March 2019 09:41:29 CET Luca Beltrame wrote:
> > 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.
> 
> I don't fully disagree with this (although I'd have stories on nefarious
> typos even for what was supposed to be a "trivial fix"). But it becomes a
> question of trade-off, IOW "how hurtful to the project mandatory reviews?"
> vs "how hurtful to the project a central component being very regularly
> broken?".
> 
> I'd argue we're loosing more with the current state of PIM than we'd loose
> with mandatory reviews.

Review all relevant changes is nice in theory, but for this to work you need 
at least two people spending comparable amount of time on this. I wish we had 
that luxury in KDE PIM.

It works to some extend for Akonadi between Dan and David, I don't see it 
working for Laurent on KMail or for me on KItinerary, there's simply not 
enough review bandwidth there.

Also, when looking at such drastic changes we should keep in mind the amount 
of changes that go in without trouble. There's always the risk of breakage 
when we change stuff with the best intentions of improving things, we need to 
deal with that no matter how many people review a change (see the nasty 
transaction lock regression in 18.12.x Akonadi).

What failed in this specific case is not the lack of review IMHO, I'm not sure 
I would have spotted the issue from the diff. It's also not that nobody cared, 
or that people ignored the issue. The underlying problem was fixed within 62 
minutes of its occurrence according to the Git log, it was however forgotten 
to push this to the 19.04 branch.

This resulted in a single build failure in the stable build for kcontacts, 
which I (and I guess others too) did not immediately act on. That does not 
mean it's being ignored, but for a change I did not do myself the overwhelming 
majority of failures is transient (as either the author fixes it upon being 
notified, or it's a dependency issue that will resolve itself with the next 
build).

That single build failure resulted in the dependent builds failing, which 
again I did not immediately act on as the error message made me believe it's 
the a dependency issue that will resolve itself. Combined with the fact that 
this is in the stable branch, which is less active than master, I had indeed 
not realized we have a persistent issue here that nobody else felt responsible 
for until I saw this email. At that point Laurent had already fixed it btw, 
which was a matter of a simple cherry-pick from master. If I missed other 
earlier communication about this somehow, I apologize of course.

So, yes, things went wrong and it's a valid question how to improve this going 
forward. But rather than questioning the usefulness of the CI or the entire 
development workflow, how about just simply pinging the people who feel 
responsible for the affected repos? It's not like we miss every single build 
issue after all. I simply might not always manage to keep the state of 120+ 
repos in various configurations in my head, and which of those needs most 
urgent attention (I for example broke half of binary factory's Android builds 
with a kpackage change recently, despite review, and yet have to find the time 
for a proper fix for that).

Regards,
Volker


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

Re: CI system maintainability

[Kevin Ottens]

Hello,

On Thursday, 28 March 2019 10:35:37 CET Luca Beltrame wrote:
> 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.

I wouldn't call that iron grip from one day to the other though. It's not like 
we've not been working toward mandatory review for the past ten years or so. 
Most KDE projects de facto apply mandatory reviews AFAICT.

Regards.
-- 
Kevin Ottens, http://ervin.ipsquad.net


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

[Kevin Ottens]

Hello,

On Thursday, 28 March 2019 10:08:54 CET Luca Beltrame wrote:
> 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.

I'm almost tempted to reply "been there, done that". It's kind of the 
situation we have today.
 
> 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)

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.

Regards.
-- 
Kevin Ottens, http://ervin.ipsquad.net


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

[Tomaz Canabrava]

On Thu, Mar 28, 2019 at 10:09 AM Luca Beltrame  wrote:
>
> 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.

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.
I strongly advocate for "reviews always", but since I was outvoted a
few times on this I would at least say "can we at least have reviews
for non project members" ?


> --
> Luca Beltrame
> GPG key ID: A29D259B

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

[Kevin Ottens]

Hello,

On Thursday, 28 March 2019 09:41:29 CET Luca Beltrame wrote:
> 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.

I don't fully disagree with this (although I'd have stories on nefarious typos 
even for what was supposed to be a "trivial fix"). But it becomes a question 
of trade-off, IOW "how hurtful to the project mandatory reviews?" vs "how 
hurtful to the project a central component being very regularly broken?".

I'd argue we're loosing more with the current state of PIM than we'd loose 
with mandatory reviews.

Regards.
-- 
Kevin Ottens, http://ervin.ipsquad.net


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

[Kevin Ottens]

Hello,

On Thursday, 28 March 2019 09:16:11 CET Ben Cooksley wrote:
> Please note that the commits in this instance were pushed without
> review, so restrictions on merge requests wouldn't make a difference
> in this case unfortunately.

Maybe it's about time to make reviews mandatory... I know it's unpopular in 
KDE, and I advocated for "don't force a tool if you can get someone to look 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.

Regards.
-- 
Kevin Ottens, http://ervin.ipsquad.net


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

Re: CI system maintainability

[Ben Cooksley]

On Thu, Mar 28, 2019 at 7:56 PM Konstantin Kharlamov  wrote:
>
>
>
> On Чт, Mar 28, 2019 at 19:40, Ben Cooksley  wrote:
> > Hi all,
> >
> > We currently have a rather substantial issue, in that the CI system
> > has been once again left in a position where it isn't possible to make
> > any changes to the system.
> >
> > This means we can't update to newer versions of packages, add new
> > packages or correct for binary incompatible changes which periodically
> > get introduced to non-Frameworks.
> >
> > This issue has arisen because currently we have a recurring failure to
> > build from source, within KDE PIM. Specifically, KContacts fails due
> > to broken CMake logic. Despite this breakage having been in place for
> > several days now, and the relevant mailing list being informed
> > automatically by the CI system, the issue has not been corrected.
> >
> > While the most immediate fix is to correct this failure to build from
> > source, that is only a short term fix and does not fix the underlying
> > issue which makes the CI system difficult to maintain - and that is
> > build failure reports being ignored, and people pushing broken code
> > that doesn't even build.
> >
> > (For those wondering, the CI system uses OpenSUSE Tumbleweed, a
> > rolling release distribution, for it's builds, so it isn't a case of
> > old CMake or anything along those lines)
> >
> > We therefore need a long term fix for this. Note that pre-commit (as
> > part of review) CI is not a solution in this instance, as the
> > offending commits did not go through review.
> >
> > Does anyone have any ideas for a long term, proper fix to this?
> >
> > At this point given the amount of effort required to maintain a CI
> > system vs. the amount of care actually being given by some developers
> > (who are ignoring it's failure emails) it becomes questionable whether
> > the effort is worth the return (and if not, we should just shut it
> > down)
> >
> > Regards,
> > Ben Cooksley
> > KDE Sysadmin
>
> I don't know abut the current CI, but judging by recent discussion that
> is about KDE migrating to gitlab; quick search shows gitlab does allow
> prohibiting a merge if CI failed¹
>
> Note however, in my experience of contributing to diffrent project CI
> often fails for reasons absolutely irrelevant to code being tested
> (e.g. errors in a CI script), in this case prohibiting a merge may
> worsen the situation.

Please note that the commits in this instance were pushed without
review, so restrictions on merge requests wouldn't make a difference
in this case unfortunately.

>
> 1:
> https://docs.gitlab.com/ee/user/project/merge_requests/merge_when_pipeline_succeeds.html#only-allow-merge-requests-to-be-merged-if-the-pipeline-succeeds
>
>

Cheers,
Ben

D19822: [Task Manager] Toggle mute when the audio indicator is clicked

[Filip Fila]

filipf added a comment.


  > In D19822#439336 , @filipf wrote:
  > 
  >> As for options overload, based on the reasoning above and the fact that 
we've recently split up the options and that we have space, I don't agree that 
this should be approached through the prism of UI clutter.
  > 
  > 
  > "We had so many options we went from one page to two pages, so now we can 
add more options!" :-)
  
  Yeah but they're good options, great options, the best of options!
  
  Just kidding, let's find work on making this feature a bit more dummy-proof 
then.
  
  Here's the code I have for color:
  
  - I removed edits in Task.qml
  - and in AudioStream.qml I did:
  
  `
  
Kirigami.Icon { // replaces IconItem at line 106; need to import Kirigami 
as well
id: audioStreamIcon
anchors.fill: parent
color: mouseArea.containsMouse? theme.negativeTextColor : 
theme.textColor
}

MouseArea {
id: mouseArea
anchors.fill: parent
hoverEnabled: true
enabled: parent.shown
onClicked: toggleMuted()
}
  
  `

REPOSITORY
  R119 Plasma Desktop

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

To: faridb, #plasma, #vdg, ngraham, filipf, hein
Cc: rooty, hein, filipf, mvourlakos, ngraham, broulik, plasma-devel, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D19822: [Task Manager] Toggle mute when the audio indicator is clicked

[Eike Hein]

hein added a comment.


  In D19822#439336 , @filipf wrote:
  
  > As for options overload, based on the reasoning above and the fact that 
we've recently split up the options and that we have space, I don't agree that 
this should be approached through the prism of UI clutter.
  
  
  "We had so many options we went from one page to two pages, so now we can add 
more options!" :-)

REPOSITORY
  R119 Plasma Desktop

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

To: faridb, #plasma, #vdg, ngraham, filipf, hein
Cc: rooty, hein, filipf, mvourlakos, ngraham, broulik, plasma-devel, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

CI system maintainability

[Ben Cooksley]

Hi all,

We currently have a rather substantial issue, in that the CI system
has been once again left in a position where it isn't possible to make
any changes to the system.

This means we can't update to newer versions of packages, add new
packages or correct for binary incompatible changes which periodically
get introduced to non-Frameworks.

This issue has arisen because currently we have a recurring failure to
build from source, within KDE PIM. Specifically, KContacts fails due
to broken CMake logic. Despite this breakage having been in place for
several days now, and the relevant mailing list being informed
automatically by the CI system, the issue has not been corrected.

While the most immediate fix is to correct this failure to build from
source, that is only a short term fix and does not fix the underlying
issue which makes the CI system difficult to maintain - and that is
build failure reports being ignored, and people pushing broken code
that doesn't even build.

(For those wondering, the CI system uses OpenSUSE Tumbleweed, a
rolling release distribution, for it's builds, so it isn't a case of
old CMake or anything along those lines)

We therefore need a long term fix for this. Note that pre-commit (as
part of review) CI is not a solution in this instance, as the
offending commits did not go through review.

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

At this point given the amount of effort required to maintain a CI
system vs. the amount of care actually being given by some developers
(who are ignoring it's failure emails) it becomes questionable whether
the effort is worth the return (and if not, we should just shut it
down)

Regards,
Ben Cooksley
KDE Sysadmin

D20086: Fix window height of Screen Locking KCM

[Tigran Gabrielyan]

tigrang created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
tigrang requested review of this revision.

REVISION SUMMARY
  An implicitHeight must be set on QML components to prevent window size being 
too small.
  
  BUG: 400355

TEST PLAN
  1. Open Screen Locking settings
  2. Go to Appearance tab
  3. Two rows of thumbnails with labels should be shown
  
  F6727233: screenlocker_kcm.png 

REPOSITORY
  R133 KScreenLocker

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

AFFECTED FILES
  kcm/wallpaperconfig.qml

To: tigrang
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart