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.