Re: EBN Still Needed?
On Mittwoch, 27. März 2019 11:26:58 CET Robby Stephenson wrote: > On Tue, Mar 26, 2019 at 4:15 PM Allen Winter wrote: > > I was notified today that the Krazy runs on the EBN have been stuck (due > > to a stale lockfile) > > for over 3 months. Is this an indication that nobody looks at the EBN > > reports any longer? > > I check EBN sporadically, not regularly. Probably on a quarter-ish basis > for Tellico. I appreciate its usefulness. Same here for LabPlot. We check krazy reports every couple of months and especially when we're close to the next release. krazy usually provides some additional value on top of clazy, tidy, coverity, cppcheck, valgrind and asan. I find this tool useful. Maybe simply remove those parts from krazy that are covered nowedays by clazy to reduce the effort for the maintanability of this tool? -- Alexander
Re: CI system maintainability
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
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
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
+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
On Fri, Mar 29, 2019 at 10:35 PM David Faure wrote: > > On jeudi 28 mars 2019 16:56:33 CET laurent Montel wrote: > > 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). > > I think the solution to all this is quite simple. If you don't want the > community to impose mandatory code reviews on you, you need to make it part of > your daily workflow to look at the state of CI for KDEPIM. > > If you go to build.kde.org, Applications, Everything kf5-qt5, and sort by > status, you can see what's currently broken (red = compilation broken, yellow > = unittests failing). > > I do this (on a frequency matching my own contributions) for all of > Frameworks, so I'm often the one pinging others about broken unittests. > Someone (who is not Ben) needs to do this for PIM, and as the most frequent > contributor, it would make sense for you to do it -- you'd often catch your > own breakages that way :) > > @Ben : do you think it would be possible to have a PIM view on build.kde.org, > with only the kde/pim/* repos? That's theoretically possible, but also non-trivial. I'll have a think about how to best implement this. > > (I also wish there was a linux-only view, given that Windows and FreeBSD have > their own set of problems. Not that I want to ignore them, I did fix things in > KF5 for those platforms - but once we get to a completely green state on Linux > (which typically happens first), it would be extremely useful to be able to > check in one glance that it stays that way.) This now exists :) https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.10/ > > @Laurent : do you also have permissions to log into build.kde.org and trigger > a build? I find this useful, to fix temporary CI problems, like > https://build.kde.org/job/Applications/view/Everything%20-%20kf5-qt5/job/ > korganizer/job/stable-kf5-qt5%20SUSEQt5.10/24/console > which failed with a weird "OSError: [Errno 26] Text file busy: '/home/ > jenkins//install-prefix/bin/kdeinit5'". That is indeed a very weird error. I'm not sure why that would have happened (best guess: overlayfs had a glitch, or the system ran out of disk space which can definitely happen) > Going one level up, I can click on, well, "Lancer un build" (why is this damn > thing in French --- well, Frenglish? ;) ), to give it another chance. Seems like Jenkins respects/follows (or at least tries) your browser language preferences :) > > -- > David Faure, fa...@kde.org, http://www.davidfaure.fr > Working on KDE Frameworks 5 > > > Cheers, Ben
Re: CI system maintainability
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
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
Re: CI system maintainability
On jeudi 28 mars 2019 16:56:33 CET laurent Montel wrote: > 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). I think the solution to all this is quite simple. If you don't want the community to impose mandatory code reviews on you, you need to make it part of your daily workflow to look at the state of CI for KDEPIM. If you go to build.kde.org, Applications, Everything kf5-qt5, and sort by status, you can see what's currently broken (red = compilation broken, yellow = unittests failing). I do this (on a frequency matching my own contributions) for all of Frameworks, so I'm often the one pinging others about broken unittests. Someone (who is not Ben) needs to do this for PIM, and as the most frequent contributor, it would make sense for you to do it -- you'd often catch your own breakages that way :) @Ben : do you think it would be possible to have a PIM view on build.kde.org, with only the kde/pim/* repos? (I also wish there was a linux-only view, given that Windows and FreeBSD have their own set of problems. Not that I want to ignore them, I did fix things in KF5 for those platforms - but once we get to a completely green state on Linux (which typically happens first), it would be extremely useful to be able to check in one glance that it stays that way.) @Laurent : do you also have permissions to log into build.kde.org and trigger a build? I find this useful, to fix temporary CI problems, like https://build.kde.org/job/Applications/view/Everything%20-%20kf5-qt5/job/ korganizer/job/stable-kf5-qt5%20SUSEQt5.10/24/console which failed with a weird "OSError: [Errno 26] Text file busy: '/home/ jenkins//install-prefix/bin/kdeinit5'". Going one level up, I can click on, well, "Lancer un build" (why is this damn thing in French --- well, Frenglish? ;) ), to give it another chance. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: CI system maintainability
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
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: EBN Still Needed?
On Mittwoch, 27. März 2019 11:26:58 CET Robby Stephenson wrote: > On Tue, Mar 26, 2019 at 4:15 PM Allen Winter wrote: > > > I was notified today that the Krazy runs on the EBN have been stuck (due > > to a stale lockfile) > > for over 3 months. Is this an indication that nobody looks at the EBN > > reports any longer? > > > > I check EBN sporadically, not regularly. Probably on a quarter-ish basis > for Tellico. I appreciate its usefulness. This is basically the usage pattern for me in KMyMoney as well. > But has the EBN code and documentation checking service out lived its > > usefulness? > > Shut it down? > > > > I would say no, unless it's truly a burden. +1 I also like the idea, which has been mentioned earlier, to integrate it into CI if possible. Thomas -- Regards Thomas Baumgart https://www.signal.org/ Signal, the better WhatsApp - 'Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it.' - Brian W. Kernighan - signature.asc Description: This is a digitally signed message part.
Re: CI system maintainability
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
[cutting the cross-post by half] On jeudi 28 mars 2019 16:45:51 CET Daniel Vrátil wrote: > 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 have been more involved in akonadi lately, and I volunteer to review your patches. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: CI system maintainability
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.
