Re: Using Gerrit for code review in KDE
On Tuesday 09 September 2014 20:02:55 Aaron J. Seigo wrote: > In any case, can you see the inconsistency between saying "we need highly > active repos to find pain points" and "these projects will only use it on an > opt-in basis, and not even for all patches"? You may as well throw a more > lightly developed repository at it all-in to get that same level of > activity. Either that or kio and plasma-framework will go all-in and then > it is exactly like the gitlab experiment. You really can't argue it both > ways. OK, let me put it differently I would expect (I obviously don't control them though) the core contributors of those two frameworks to go all in for a while. Where I see a difference is that during the time of experiment it will leave the other less committed contributors undisturbed. > A reasonable project of a 3-5 active developers who are doing 2-3 patch sets > a week ought to give one all the data set necessary. One can extrapolate > from such a sample pretty easily. I know I managed to do it with gitlab and > there wasn't even that level of activity. I agree that makes sense too. Now there was a will in the room of trying out with a couple of frameworks. Right now we seem to be in a phase active vs almost inactive, so people chose two active ones. > So as I asked, are there any actively developed repos that aren't *as* > critical and part of major stable releases? That ought to be a rhetorical > question as I can think of probably half a dozen off the top of my head. How > about that new video player that was demo'd at akademy? How about > kde-connect? How about the plasma-desktop repository, if the plasma team > really wants to try it out? Sure, all valid ones to try with IMO. > Why go straight for stable, releasing frameworks? Basically the topic was brought up to that particular BoF by Jan which in turn prompted the "OK, this installation is low risk, let's try with two frameworks to test the water". Part of the reasoning there has also been "the tool might give us further ideas on our branch policies", since there's some KF specific decisions around those due to the one month cycle it was considered to make sense to have KF part of the experiment. > > I then doubt it would be a problem for new developers. The only thing they > > would "loose" by default is the knowledge of some of the patches cooking > > up in Gerrit when the team tests it. But I would be surprised if the > > majority of new developers actively look at the list of patches prepared > > in RB either. > > I'd think about the will-be-long-term-active minority for which one ought to > keep the barrier to entry low which includes consistency in tooling Agreed, but that's where I don't follow you. Those one shouldn't see a difference in the barrier of entry during the experiment. > and a reasonable measure of project-follows-documented-processes. That I can agree with that indeed for a limited time some people in the project will go through a specific review tool. That said our policies in KF ask for reviews, they don't mandate a particular tool. It was done to open the door to reviews by email and pastebin. It is no different IMO. > I'd also think about the developers who follow those reviews now and who > will now have to pay attention to multiple tools to keep up. Those were in the room and willing to pay that price for the time of the experiment. > I'm all for improvements through change, but one can minimize disruptions as > they go. Sure. Still I fail to perceive a large disruption in that case. I'd agree if we said "OK, all contributions to kio go through Gerrit now", we're saying "during the time of the experiment core devel contributions to kio go through Gerrit, everything else is business as usual". Regards. -- Kévin Ottens, http://ervin.ipsquad.net KDAB - proud supporter of KDE, http://www.kdab.com signature.asc Description: This is a digitally signed message part.
Re: Review Request 120119: KRecursiveFilterProxyModel: Fixed the model
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120119/ --- (Updated Sept. 10, 2014, 8:15 a.m.) Review request for kdelibs and Stephen Kelly. Changes --- added not that this applies to KF5 as well. Bugs: 338950 http://bugs.kde.org/show_bug.cgi?id=338950 Repository: kdelibs Description (updated) --- (This problem probably applies to KF5 as well, and we'll need to forward port this patch.) KRecursiveFilterProxyModel: Fixed the model The model was not working properly and didn't include all items under some circumstances. This patch fixes the following scenarios in particular: * The change in sourceDataChanged is required to fix the shortcut condition. The idea is that if the parent is already part of the model (it must be if acceptRow returns true), we can directly invoke dataChanged on the parent, resulting in the changed index getting reevaluated. However, because the recursive filterAcceptsRow version was used the shortcut was also used when only the current index matches the filter and the parent index is in fact not yet in the model. In this case we failed to call dataChanged on the right index and thus the complete branch was never added to the model. * The change in refreshAscendantMapping is required to include indexes that were included by descendants. The intended way how this was supposed to work is that we traverse the tree upwards and find the last index that is not yet part of the model. We would then call dataChanged on that index causing it and its descendants to get reevaluated. However, acceptRow does not reflect wether an index is already in the model or not. Consider the following model: - A - B - C - D If C is include in the model by default but D not and A & B only gets included due to C, we have the following model: - A - B - C - D If we then call refreshAscendantsMapping on D it will not consider B as already being part of the model. This results in the toplevel index A being considered lastAscendant, and a call to dataChanged on A results in a reevaluation of A only, which is already in the model. Thus D never gets added to the model. Unfortunately there is no way to probe QSortFilterProxyModel for indexes that are already part of the model. Even the const mapFromSource internally creates a mapping when called, and thus instead of revealing indexes that are not yet part of the model, it silently creates a mapping (without issuing the relevant signals!). As the only possible workaround we have to issues dataChanged for all ancestors which is ignored for indexes that are not yet mapped, and results in a rowsInserted signal for the correct indexes. It also results in superfluous dataChanged signals, since we don't know when to stop, but at least we have a properly behaving model this way. Diffs - kdeui/itemviews/krecursivefilterproxymodel.cpp 6d6563166bcc9637d826f577925c47d5ecbef2cd kdeui/tests/CMakeLists.txt f661b9177a6e0e1de7f49bc3cb9fbb5e04f427c1 kdeui/tests/krecursivefilterproxymodeltest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/120119/diff/ Testing --- Thanks, Christian Mollekopf
Re: Using Gerrit for code review in KDE
On Wednesday, September 10, 2014 00.23:18 Jan Kundrát wrote: > On Tuesday, 9 September 2014 21:44:25 CEST, Alexander Neundorf wrote: > > Having two different patch review systems for one project... I > > mean, this is > > surely not a good idea. Two places to send patches, to places to review > > patches, two different user interfaces, > > This is not a final state. To be honest, I think that having a flag-day > migration would be even worse, so running with two competing systems for a > while seems like the only plausible way to proceed here -- otherwise one > has that annoying chicken-and-egg problem. talking about a migration, "flag-day" or otherwise, before proving runs have been done is premature. this seems to be at the test-it-out stage, and as such it would be nice to see it not disrupt extant and documented workflow. imagine the disruption that would ensue if we took this approach for every experiment that alters extant practice. i understand you are a great advocate for gerrit, and that's cool; but please bring it into kde with respect for the big picture and do so with responsibility. i think you'll find it a lot easier to convince people it is a good idea if there is a measured, responsible approach applied. > Please note that some of the often-quoted UI and usability glitches have it really has very little to do with how good gerrit is. gerrit can be the messiah sent from on high, but continuity in development processes across the community is even more important. there are ways to introduce workflow improvements, and the options vary depending on whether there is precedent, documentation, etc. anyways .. i've said my bit, so EOT for me :) cheers ... -- Aaron J. Seigo signature.asc Description: This is a digitally signed message part.
Review Request 120127: Don't show overwrite dialog if file name is empty
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120127/ --- Review request for kdelibs. Repository: kdelibs Description --- Programs using setConfirmOverwrite(true) (e.g. LibreOffice with KDE4 integration, Mozilla products with KDE4 integration, several components of the PIM suite) will trigger the following strange behaviour: If the file name is empty and the user selects "Save", the dialog will ask if you want to overwrite the current directory instead of just ignoring the empty input. By moving the query below the directory checks the order is correct again, showing the overwrite confirmation dialog only if the field contains a unique filename. Looking at the code this also affects Frameworks 5, though I haven't tested it yet. Diffs - kfile/kfilewidget.cpp fc9b169 Diff: https://git.reviewboard.kde.org/r/120127/diff/ Testing --- Thanks, Ignaz Forster
Re: Retiring and testament
On Tuesday 09 September 2014 17:41:59 Kevin Ottens wrote: > You will still see me around of course. I'm retiring from KDE Frameworks, > not from KDE. I just want to focus mainly on Zanshin and some of my > community work (french promo, manifesto, e.V.). Because of my other > involvements you'll probably see me sending patches from time to time to > KF5, just don't expect me to monitor closely what's going on or to drive > anything anymore. Hi Kevin Thank you for so much work you've done and for leading the KF5 effort. > Also, I am actively discussing with Kevin Krammer to pass him the torch. I > guess most of you know him. He will be a good replacement since he has a > good view on the whole stack. Not to neglect the fact we both have the > same first name so you feel right at home. Yeah, thanks for making the transition easy! The first name helps! -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358
