D20086: Fix window height of Screen Locking KCM

[Nathaniel Graham]

ngraham added a comment.


  @tigrang would you like this landed now, or do you wanna try 
@davidedmundson's idea?
  
  Thanks for the patch!

REPOSITORY
  R133 KScreenLocker

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

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

Re: CI system maintainability

[Kevin Ottens]

Hello,

On Thursday, 28 March 2019 20:35:11 CET Dr.-Ing. Christoph Cullmann wrote:
> 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.

That I hope we'll get fixed at some point. It's a big big advantage when you 
can get a CI run on reviews. I find it best when you get both humans and 
scripts looking at the code in a review. It helps a lot in having better 
quality reviews from the humans since they are relieved from the boring 
redundant nitpicking (catching warnings, basic code style, etc.).

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


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

Re: CI system maintainability

[Andrius Štikonas]

+1 for this. I think running tests before merging is more acceptable than 
having mandatory reviews.

On 29 March 2019 11:10:52 GMT+00:00, Ovidiu-Florin Bogdan 
 wrote:
>Hello,
>
>A Merge Request in GitLab does not necessarily imply the need for a
>review by e person. It can just run a pipeline to validate that the
>code isn't broken. If the pipeline fails, the merge button is not
>available.
>
>We use GitLab at work and we have it set up like this:
>
>* Main branches (develop/master/release/etc) are proteted and cannot be
>directly commited/pushed to, and only updated through MR
>* Each project defines what it's build/validate pipeline is
>(Jenkinsfile in project repo)
>* The pipeline is executed uppon creating the MR
>* if the Pipeline passes, the MR can be merged to the mainline branch
>
>This way we ensure that no code gets in that fails the build or with
>tests failing.
>
>P.S. We also store the build artifacts in a binary repository from
>where other pipelines can fetch them to be used in compiling other
>projects.
>
>P.P.S. This is the "DevOps" process used in most companies. The tools
>might differ, but the process is the same. It's the same for most FOSS
>projects as well.
>
>Regards,
>Ovidiu
>
>În ziua de joi, 28 martie 2019, la 10:29:22 EET, Kevin Ottens a scris:
>> 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.
>> 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: CI system maintainability

[Ben Cooksley]

On Fri, Mar 29, 2019 at 9:56 PM Kevin Ottens  wrote:
>
> Hello,
>
> On Thursday, 28 March 2019 20:35:11 CET Dr.-Ing. Christoph Cullmann wrote:
> > 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.
>
> That I hope we'll get fixed at some point. It's a big big advantage when you
> can get a CI run on reviews. I find it best when you get both humans and
> scripts looking at the code in a review. It helps a lot in having better
> quality reviews from the humans since they are relieved from the boring
> redundant nitpicking (catching warnings, basic code style, etc.).

With the shift to Gitlab we should be able to provide this hopefully.

We're still figuring out how to be able to provide CI in an easy to
maintain manner (in terms of controlling platforms builds are done
for, which branches, etc).
This is a non-trivial problem (which is why Jenkins is only able to do
master/trunk and stable builds currently) but it should be solvable.

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

Cheers,
Ben

Re: CI system maintainability

[Ovidiu-Florin Bogdan]

Hello,

A Merge Request in GitLab does not necessarily imply the need for a review by e 
person. It can just run a pipeline to validate that the code isn't broken. If 
the pipeline fails, the merge button is not available.

We use GitLab at work and we have it set up like this:

* Main branches (develop/master/release/etc) are proteted and cannot be 
directly commited/pushed to, and only updated through MR
* Each project defines what it's build/validate pipeline is (Jenkinsfile in 
project repo)
* The pipeline is executed uppon creating the MR
* if the Pipeline passes, the MR can be merged to the mainline branch

This way we ensure that no code gets in that fails the build or with tests 
failing.

P.S. We also store the build artifacts in a binary repository from where other 
pipelines can fetch them to be used in compiling other projects.

P.P.S. This is the "DevOps" process used in most companies. The tools might 
differ, but the process is the same. It's the same for most FOSS projects as 
well.

Regards,
Ovidiu

În ziua de joi, 28 martie 2019, la 10:29:22 EET, Kevin Ottens a scris:
> 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.
> 




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

Re: CI system maintainability

[Ben Cooksley]

On Fri, Mar 29, 2019 at 10:33 PM Friedrich W. H. Kossebau
 wrote:
>
> Am Donnerstag, 28. März 2019, 23:06:17 CET schrieb laurent Montel:
> > Le jeudi 28 mars 2019, 18:27:42 CET Friedrich W. H. Kossebau a écrit :
> > > 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 :
> > > > > 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.
>
> Do you remember a reason why it is no longer the case?
>
> IMHO bringing the build report bot back to the chat channel could help with
> the issue this thread is about.

It was quite possibly lost during one of the various refactors we've
had to do of the CI system to solve maintainability issues.
We've had a couple of generations of the CI system so far (by my
count, we're on generation 4), and things have unfortunately been lost
in between the switch between various generations.

IRC channel notifications are currently governed by the rules in
sysadmin/irc-notifications (which is also where commits and Bugzilla
activity notifications for IRC channels are controlled from). See
https://cgit.kde.org/sysadmin/irc-notifications.git/tree/jenkins/notifications.cfg

>
> People tend to look more often into the chat channel then in their email
> folder, so this bot would improve the visibility of the state on
> build.kde.org, also be in a public place so people can directly chat about
> any reasons.
>
> > > > > 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?
> >
> > I read it sometime as it arrives in my kdepim-devel mailbox, but indeed
> > sometime we can have several mail in same time because we increase a
> package
> > dependancy and it breaks all other packages. So indeed I didn"t look at all
> > the time.
> >
> > But when a people signals me a problem I try to fix it quickly.
> >
> > An email send after 1 day that package is broken can be a good idea, so it
> > can't be a dependancy problem because we increase package version just that
> > it's really broken.
>
> Increasing the package version ideally should not result in CI breakages.
> Ideally CI only fails if there is a real issue, otherwise people just get
> used to it failing and do not give the deserved attention.
>
> What would prevent you to turn to a system like used with KDE Frameworks,
> where there is some internal dependency version and a separate actual
> version, with the actual version bumped earlier and the internal dependency
> version only bumped some days later? From what I saw, you increase versions
> quite often in PIM, so related breakages would happen quite often.
>
> Cheers
> Friedrich

Cheers,
Ben

>
> PS: Okay if we start to slim the list of CC:s a bit now? Would leave out
> plasma, kdevelop, frameworks-devel on any next reply at least.
>
>

Re: CI system maintainability

[Ben Cooksley]

On Fri, Mar 29, 2019 at 6:45 AM Johannes Zarl-Zierl
 wrote:
>
> Hi,

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

While this does seem like a nice solution, unfortunately for many
repositories it isn't a case of disabling CI coverage for it, but also
CI coverage for everything that depends on it. In the case of
KContacts, this would also impact on parts of Extragear and Calligra
(who depending on their exact requirements would either lose a
dependency being available, or lose all of their CI coverage).

This is why i've not pursued this as an option in the past, because
it's not fair on other projects that don't have anything to do with
another project aside from being a user of it's interfaces to lose
their coverage, simply because the project they depend on is broken.

>
>   Johannes

Cheers,
Ben

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

D19963: Avoid serializing base64 encoded favicon data twice

[Stefan Brüns]

bruns added a comment.


  Ping!

REPOSITORY
  R856 Plasma Browser Integration

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

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

D20113: [Desktop Theme KCM] Adapt clock to new hand rotation center options

[Friedrich W. H. Kossebau]

kossebau added a comment.


  See D20112  for the same change for the 
applet.
  
  To test, you can download the Unicorn theme from store.kde.org, then edit the 
.local/share/plasma/desktoptheme/unicorn/widgets/clock.svg and append this 
before the final ``:
  





  
  Clearing the cache and then reloading the theme settings will then fix the 
Unicorn clock to look like this (minute hand properly rotated around end) as 
well keep the other clocks unchanged:
  F6730184: Screenshot_20190329_125516.png 


REPOSITORY
  R119 Plasma Desktop

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

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

D20113: [Desktop Theme KCM] Adapt clock to new hand rotation center options

[Friedrich W. H. Kossebau]

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

REVISION SUMMARY
  The new hints
  
  - hint-hourhand-rotation-center-offset
  - hint-minutehand-rotation-center-offset
  
  allow themes to define where in the hand pixmap the rotation center should
  be placed.

TEST PLAN
  Clear the cache (rm .cache/plasma* -r) and restart system settings, see
  clocks of unmodified themes still have hands at normal position, and hands
  of modified themes at expected positions.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  fixclockshadowhandcenter

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

AFFECTED FILES
  kcms/desktoptheme/package/contents/ui/Hand.qml
  kcms/desktoptheme/package/contents/ui/ThemePreview.qml

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

D20112: [analog-clock] Allow themes to define hand shadow offset & hand rot center

[Friedrich W. H. Kossebau]

kossebau added a comment.


  To test, you can download the Unicorn theme from store.kde.org, then edit the 
.local/share/plasma/desktoptheme/unicorn/widgets/clock.svg and append this 
before the final ``:
  





  
  Reloading the theme will then fix the clock to look like this (extreme shadow 
offset just done for demo purpose)
  F6730106: Screenshot_20190329_112243.png 


REPOSITORY
  R120 Plasma Workspace

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

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

D20112: [analog-clock] Allow themes to define hand shadow offset & hand rot center

[Friedrich W. H. Kossebau]

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

REVISION SUMMARY
  Currently the shadow offset of the hands as well as the rotation center of
  the hands is hardcoded to match the light model and the hands shape of the
  Breeze theme. As well did some older change to move the rotation center to
  width/2 in y direction break older themes which relied on the y=0 offset.
  
  This patch adds the option for themes to control the shadow offset as well
  as define the rotation offset for each hand (to avoid the need to create
  large pixmaps as workaround with empty space to match the assumption of
  the center to be at (width/2, width/2), by these new hints:
  
  One shadow offset for all hands, as the simulated physical model can be
  assumed to have all hands almost on same layer (also matching that we have
  only one shadow for stacked objects, like windows, elsewhere):
  
  - hint-hands-shadow-offset-to-west/hint-hands-shadows-offset-to-east
  - hint-hands-shadow-offset-to-north/hint-hands-shadow-offset-to-south
  
  (west vs. east & north vs. south as negative rect size is not possible).
  If no hint is set, defaults to 0.
  The default of 0 is a small breakage, but almost all themes from
  store.kde.org do not use a shadow, also was the hour shadow broken/not shown
  until recently and only fixed for upcoming Plasma 5.16, where the offset of
  the shadows was tuned as well,  so any themes with hand shadows will need
  some tuning for Plasma 5.16 in any case.
  
  Separate rotation offsets for all hands & their shadows, so pixmaps can be
  as small as needed, with no transparent area padding needed:
  
  - hint-hourhand-rotation-center-offset
  - hint-hourhandshadow-rotation-center-offset
  - hint-minutehand-rotation-center-offset
  - hint-minutehandshadow-rotation-center-offset
  - hint-secondhand-rotation-center-offset
  - hint-secondhandshadow-rotation-center-offset
  
  The offset is taken from by the (width,height) of the hint element.
  Sadly this does not allow to specify a 0 in any dimension, as such an item
  is not passed through from QSvgRenderer/Plasms::Svg. Most themes might not
  need that though, so the few which might have to fall back to padding with
  transparent area.
  If no hint is set, defaults to (width/2, width/2).

TEST PLAN
  Clocks of unmodified themes from store.kde.org still look as before.
  Themes with hints added have shadows at proper place as well as rotation
  center of hands at the expected offset.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  fixclockshadowhandcenter

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

AFFECTED FILES
  applets/analog-clock/contents/ui/Hand.qml
  applets/analog-clock/contents/ui/analogclock.qml

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

D19687: Allow single images to be excluded from the slideshow

[David Redondo]

davidre added a comment.


  almost weekly ping

REPOSITORY
  R120 Plasma Workspace

BRANCH
  toggleSlides

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

To: davidre, #plasma, #vdg, ngraham
Cc: filipf, abetts, 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, 23:06:17 CET schrieb laurent Montel:
> Le jeudi 28 mars 2019, 18:27:42 CET Friedrich W. H. Kossebau a écrit :
> > 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 :
> > > > 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.

Do you remember a reason why it is no longer the case?

IMHO bringing the build report bot back to the chat channel could help with 
the issue this thread is about.

People tend to look more often into the chat channel then in their email 
folder, so this bot would improve the visibility of the state on 
build.kde.org, also be in a public place so people can directly chat about 
any reasons.

> > > > 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?
> 
> I read it sometime as it arrives in my kdepim-devel mailbox, but indeed
> sometime we can have several mail in same time because we increase a 
package
> dependancy and it breaks all other packages. So indeed I didn"t look at all
> the time.
> 
> But when a people signals me a problem I try to fix it quickly.
> 
> An email send after 1 day that package is broken can be a good idea, so it
> can't be a dependancy problem because we increase package version just that
> it's really broken.

Increasing the package version ideally should not result in CI breakages. 
Ideally CI only fails if there is a real issue, otherwise people just get 
used to it failing and do not give the deserved attention.

What would prevent you to turn to a system like used with KDE Frameworks, 
where there is some internal dependency version and a separate actual 
version, with the actual version bumped earlier and the internal dependency 
version only bumped some days later? From what I saw, you increase versions 
quite often in PIM, so related breakages would happen quite often.

Cheers
Friedrich

PS: Okay if we start to slim the list of CC:s a bit now? Would leave out 
plasma, kdevelop, frameworks-devel on any next reply at least.

Re: CI system maintainability

[Kevin Ottens]

Hello,

On Friday, 29 March 2019 09:43:44 CET Volker Krause wrote:
> On Friday, 29 March 2019 08:59:59 CET Kevin Ottens wrote:
> > On Thursday, 28 March 2019 21:53:06 CET Alexander Neundorf wrote:
> > > Having mandatory reviews for a central and complex component like
> > > akonadi
> > > looks like a very good and obvious idea.
> > 
> > Yep.
> 
> Looking at the 18.12 -> 19.04 timeframe the majority of changes to Akonadi
> went through pre-commit review, even more so if you discard commits doing
> release work (version bumps etc) or similar maintenance not touching the
> actual logic.
> 
> And specifically the changes that caused us the most headaches due to
> introducing a nasty regression went through review.
> 
> Sure, nothing is perfect, but I don't think code review in Akonadi is the
> most pressing issue here.

Fair enough. I was thinking more PIM in general though than Akonadi in 
particular.
 
> > > OTOH if there is only one developer who is really expert for akonadi,
> > > this makes it kind of unfeasible.
> > 
> > That's the chicken and egg problem we're in concerning KDEPIM. The
> > developer story is frankly really harder than most software out there
> > which makes it unlikely for people to pick it over something else for
> > contributions. That's in part tied to your next point below and partly
> > tied to
> > documentation, on- boarding etc. The unwillingness to be slowed down is
> > getting in the way of fixing that situation: to be a desirable project to
> > contribute to you need to spend time advocating, documenting and taking
> > newbies by the hand until they become regular contributors.
> > 
> > Yes it's tough, and TBH I'm guilty of not doing this more on my own
> > projects. But on such a strategic piece of software like KDEPIM there's
> > some responsibility in carrying those duties for the well being of the
> > project.
> 
> How to address the issue of bus factor ~1 components in PIM is the real
> question here, I completely agree. But this is getting way off topic from
> Ben's original issue, and for the wide range of recipients.

Yes, I realized only too late that I kind of hijacked the thread somehow. I 
apologies about that.

> Also, I don't think overly generic statements on that help us much, so maybe
> let's discuss concrete steps for this at the sprint next week?

Definitely. It's in part because I know the sprint is coming that I started to 
wave that particular flag. :-)

I wish Laurent was there though, it'll make that particular discussion harder 
to conclude without him...

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


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

Re: CI system maintainability

[Volker Krause]

On Friday, 29 March 2019 08:59:59 CET Kevin Ottens wrote:
> On Thursday, 28 March 2019 21:53:06 CET Alexander Neundorf wrote:
> > Having mandatory reviews for a central and complex component like akonadi
> > looks like a very good and obvious idea.
> 
> Yep.

Looking at the 18.12 -> 19.04 timeframe the majority of changes to Akonadi 
went through pre-commit review, even more so if you discard commits doing 
release work (version bumps etc) or similar maintenance not touching the 
actual logic.

And specifically the changes that caused us the most headaches due to 
introducing a nasty regression went through review.

Sure, nothing is perfect, but I don't think code review in Akonadi is the most 
pressing issue here.
 
> > OTOH if there is only one developer who is really expert for akonadi, this
> > makes it kind of unfeasible.
> 
> That's the chicken and egg problem we're in concerning KDEPIM. The developer
> story is frankly really harder than most software out there which makes it
> unlikely for people to pick it over something else for contributions.
> That's in part tied to your next point below and partly tied to
> documentation, on- boarding etc. The unwillingness to be slowed down is
> getting in the way of fixing that situation: to be a desirable project to
> contribute to you need to spend time advocating, documenting and taking
> newbies by the hand until they become regular contributors.
> 
> Yes it's tough, and TBH I'm guilty of not doing this more on my own
> projects. But on such a strategic piece of software like KDEPIM there's
> some responsibility in carrying those duties for the well being of the
> project.

How to address the issue of bus factor ~1 components in PIM is the real 
question here, I completely agree. But this is getting way off topic from 
Ben's original issue, and for the wide range of recipients. 

Also, I don't think overly generic statements on that help us much, so maybe 
let's discuss concrete steps for this at the sprint next week?

Regards,
Volker


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

Re: CI system maintainability

[Kevin Ottens]

Hello,

On Thursday, 28 March 2019 21:53:06 CET Alexander Neundorf wrote:
> On 2019 M03 28, Thu 16:04:01 CET Boudhayan Gupta wrote:
> > 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.)

Hell yeah, it's not a silver bullet. Actually it can be only one safety net 
among others. None of them are perfect, none of them will catch it all or be 
of good quality all the time, it's just about mitigating risks as much as 
possible.
 
> > 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.

I don't think that's true. It's a question of complexity in the system really. 
In a trivial system indeed they are less likely to introduce bugs than for 
bigger patches... but as the software grows and complexity rises (especially 
with the multiplication of mutable states) one liners tend to be as error-
prone as 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.

Yep.

> OTOH if there is only one developer who is really expert for akonadi, this
> makes it kind of unfeasible.

That's the chicken and egg problem we're in concerning KDEPIM. The developer 
story is frankly really harder than most software out there which makes it 
unlikely for people to pick it over something else for contributions. That's 
in part tied to your next point below and partly tied to documentation, on-
boarding etc. The unwillingness to be slowed down is getting in the way of 
fixing that situation: to be a desirable project to contribute to you need to 
spend time advocating, documenting and taking newbies by the hand until they 
become regular contributors.

Yes it's tough, and TBH I'm guilty of not doing this more on my own projects. 
But on such a strategic piece of software like KDEPIM there's some 
responsibility in carrying those duties for the well being of the project.

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

Yes, it's byzantine really. And the user experience is still not great (to be 
polite), had to setup some new hardware recently and I was honestly almost to 
the point of throwing it all through the window.

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


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