Review Request 120178: Set the kio_http responsecode metadata on error
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120178/ --- Review request for kdelibs. Bugs: 337198 http://bugs.kde.org/show_bug.cgi?id=337198 Repository: kdelibs Description --- The attached patch sets the HTTP responsecode metadata on error. Diffs - kioslave/http/http.cpp 1068eb0e7780dd02f3af284e5d1ba932c06f4e1f Diff: https://git.reviewboard.kde.org/r/120178/diff/ Testing --- Thanks, Dawit Alemayehu
Review Request 120182: KIO::CopyJob: Do not query for free space when filesystem type is unknown
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120182/ --- Review request for kdelibs and David Faure. Bugs: 336529 http://bugs.kde.org/show_bug.cgi?id=336529 Repository: kdelibs Description --- This patch stops KIO::CopyJob from querying for free disk space when the filesystem type is unknown. Diffs - kio/kio/copyjob.cpp 713255b123d284bbbaab67073466605efdd96aee Diff: https://git.reviewboard.kde.org/r/120182/diff/ Testing --- Mounted Andriod filesystem through sshfs and attempted to copy files through sftp. Thanks, Dawit Alemayehu
Re: Using Gerrit for code review in KDE
El Divendres, 12 de setembre de 2014, a les 22:52:40, Marco Martin va escriure: On Tuesday, September 9, 2014, Jan Kundrát j...@flaska.net wrote: If you would like all plasma to go, just give me a list of repos and I can make it happen. No, definitely not yet In my opinion, the purpose of this test is not to verify that Gerrit works or that the ACLs are set up properly -- both were done already. As part of the experiment i would also like to try to have stricter acls for +2 and submit, like starting from mantainers then slowly adding people (that's also how i understood it would have worked during the bof) I'd read that as being against the KDE Manifesto. Cheers, Albert -- Marco Martin
Re: Re: Using Gerrit for code review in KDE
On Saturday 13 September 2014 16:51:15 Albert Astals Cid wrote: El Divendres, 12 de setembre de 2014, a les 22:52:40, Marco Martin va escriure: On Tuesday, September 9, 2014, Jan Kundrát j...@flaska.net wrote: If you would like all plasma to go, just give me a list of repos and I can make it happen. No, definitely not yet In my opinion, the purpose of this test is not to verify that Gerrit works or that the ACLs are set up properly -- both were done already. As part of the experiment i would also like to try to have stricter acls for +2 and submit, like starting from mantainers then slowly adding people (that's also how i understood it would have worked during the bof) I'd read that as being against the KDE Manifesto. my understanding was that it's still possible to bypass the code review, so I cannot see that it's against the KDE Manifesto as it's only a kind of social contract. Or am I missing something. Personally I like the idea of having the +2 limited to the devs familiar with the code. Cheers Martin signature.asc Description: This is a digitally signed message part.
Administrating project boards on todo.kde.org
Hey all, I hope this is the right place to ask. I would like to start using todo.kde.org more. It's imo a good place to track jobs that need to be done. I did not figure out how to add categories though. Can we somehow give project admins (see projects.kde.org) the required rights to that website? Or simply allow every KDE account to create a new subject and/or board? Bye -- Milian Wolff m...@milianw.de http://milianw.de
Re: Administrating project boards on todo.kde.org
2014-09-13 14:25 GMT-03:00 Milian Wolff m...@milianw.de: Hey all, I hope this is the right place to ask. I would like to start using todo.kde.org more. It's imo a good place to track jobs that need to be done. I did not figure out how to add categories though. Can we somehow give project admins (see projects.kde.org) the required rights to that website? Or simply allow every KDE account to create a new subject and/or board? I just granted you admin access on todo.kde.org. Regards, -- Nicolás KDE Sysadmin Team. -ish.
Re: Using Gerrit for code review in KDE
On 13.09.2014 17:49, Martin Gräßlin wrote: my understanding was that it's still possible to bypass the code review, so I cannot see that it's against the KDE Manifesto as it's only a kind of social contract. Or am I missing something. Personally I like the idea of having the +2 limited to the devs familiar with the code. I *strongly* disagree with and even object to this. One of the biggest achievements of the KDE community is having become multi-generational and turnover-proof, whereas most pro- jects peter out when their initial developer generation moves on. Note how few people in this thread were around in 1998. I attribute this to a few traits of our structure: * Flat hierarchies and few, mostly informally held titles. * Broad, equal write access. * Encouraging people to feel responsible for what we make. These things reinforce each other in multiple ways. If main- tainers are not entrenched positions, they're easy to replace when they move on (whether they can accept this themselves or not). Once you codify them in ACLs (and yes, we do this in some places already, and I think this was a mistake as well) you add inertia because those ACLs need to be updated, and someone needs to work up the gumption to ask for that update. (Case study of what can happen when we lose track of this: We lost KDM. There are many more positive case studies where contributors kept projects alive once maintainers disappeared just by slowly ramping up their involvement, without needing hand over the keys flag days.) If maintainers aren't entrenched, you also can't rely on them picking up the slack; hence encouraging stepping up. How do you become a maintainer? By actively doing review, for one. I.e. it's up to *maintainers* to stay active and involve themselves in the process, and provide guidance so that good code goes in and bad code stays out. The safety net we have for review working is our brains when making or picking through arguments. The argument but you can still bypass Gerrit and push directly, this is just social etiquette doesn't matter because the whole thing is about social etiquette. The ACLs we already have reflect our social etiquette, and that means we need to be careful and think smart about evolving it. Personally I think social etiquette encouraging the use of review tools is fine. Social etiquette that entrenches some developers as special on those review tools is not. Cheers, Eike
Re: Using Gerrit for code review in KDE
On Saturday, 2014-09-13, 17:49:31, Martin Gräßlin wrote: On Saturday 13 September 2014 16:51:15 Albert Astals Cid wrote: El Divendres, 12 de setembre de 2014, a les 22:52:40, Marco Martin va escriure: On Tuesday, September 9, 2014, Jan Kundrát j...@flaska.net wrote: If you would like all plasma to go, just give me a list of repos and I can make it happen. No, definitely not yet In my opinion, the purpose of this test is not to verify that Gerrit works or that the ACLs are set up properly -- both were done already. As part of the experiment i would also like to try to have stricter acls for +2 and submit, like starting from mantainers then slowly adding people (that's also how i understood it would have worked during the bof) I'd read that as being against the KDE Manifesto. my understanding was that it's still possible to bypass the code review, so I cannot see that it's against the KDE Manifesto as it's only a kind of social contract. Or am I missing something. That would be my interpretation as well. Also, I think our current albeit unwritten social rules or hacker ethics kind of do something similar already. I.e. if something gets committed that a maintainer does not approve of and reverts, then it stays reverted. The choice to make the maintainer's role more explicit throught the tooling is just making the decision more active than reactive. As for submit, that IMHO should at least also be available to the review request owner. Does anyone see advantages of having submit restricted at all once the necessary approval has been achieved? Cheers, Kevn -- Kevin Krammer, KDE developer, xdg-utils developer KDE user support, developer mentoring signature.asc Description: This is a digitally signed message part.
Re: Using Gerrit for code review in KDE
On 13.09.2014 20:21, Ivan Čukić wrote: I agree, +2 should be retained by the maintainer, or a smaller set of developers as decided by the maintainer. Or perhaps it simply turns out that the whole idea of *having* a '+2' is incompatible with the KDE community in the first place. Do we really want arbitrary design specifics of a tool to dictate how we work together? Cheers, Eike
Re: Administrating project boards on todo.kde.org
On Saturday 13 September 2014 14:38:03 Nicolás Alvarez wrote: 2014-09-13 14:25 GMT-03:00 Milian Wolff m...@milianw.de: Hey all, I hope this is the right place to ask. I would like to start using todo.kde.org more. It's imo a good place to track jobs that need to be done. I did not figure out how to add categories though. Can we somehow give project admins (see projects.kde.org) the required rights to that website? Or simply allow every KDE account to create a new subject and/or board? I just granted you admin access on todo.kde.org. Thanks. But, in general, shouldn't more people get rights on that page? Do we really need to request the rights to create a new board from the sysadmins? Same for categories? Bye -- Milian Wolff m...@milianw.de http://milianw.de
Re: Using Gerrit for code review in KDE
On Saturday, 2014-09-13, 20:38:21, Eike Hein wrote: These things reinforce each other in multiple ways. If main- tainers are not entrenched positions, they're easy to replace when they move on (whether they can accept this themselves or not). Once you codify them in ACLs (and yes, we do this in some places already, and I think this was a mistake as well) you add inertia because those ACLs need to be updated, and someone needs to work up the gumption to ask for that update. (Case study of what can happen when we lose track of this: We lost KDM. There are many more positive case studies where contributors kept projects alive once maintainers disappeared just by slowly ramping up their involvement, without needing hand over the keys flag days.) Hmm these are good points. I guess most people commenting so far have done so from a mostly KDE frameworks point of view, where maintainers want to be actively involved instead of having to reactively revert changes that don't fit or need more discussion. So your suggestion would be to not have +2 but a policy of some sort that only the +1 of the maintainer, if there is an active one, is considered as go ahead? If maintainers aren't entrenched, you also can't rely on them picking up the slack; hence encouraging stepping up. How do you become a maintainer? By actively doing review, for one. I.e. it's up to *maintainers* to stay active and involve themselves in the process, and provide guidance so that good code goes in and bad code stays out. The safety net we have for review working is our brains when making or picking through arguments. That sounds similar to Qt's approvers. The argument but you can still bypass Gerrit and push directly, this is just social etiquette doesn't matter because the whole thing is about social etiquette. The ACLs we already have reflect our social etiquette, and that means we need to be careful and think smart about evolving it. Personally I think social etiquette encouraging the use of review tools is fine. Social etiquette that entrenches some developers as special on those review tools is not. If I brainstorm about alternatives I find: * let maintainers have -2 as pro-active variation of reverting * build social ettiquette to always wait for the maintainer's +1 but at most for a certain grace period, e.g. one week * have everyone get +2 and use etiquette to do that only if there is already strong agreement or the grace period has passed Other? Cheers, Kevin -- Kevin Krammer, KDE developer, xdg-utils developer KDE user support, developer mentoring signature.asc Description: This is a digitally signed message part.
Re: Administrating project boards on todo.kde.org
2014-09-13 15:57 GMT-03:00 Milian Wolff m...@milianw.de: On Saturday 13 September 2014 14:38:03 Nicolás Alvarez wrote: 2014-09-13 14:25 GMT-03:00 Milian Wolff m...@milianw.de: Hey all, I hope this is the right place to ask. I would like to start using todo.kde.org more. It's imo a good place to track jobs that need to be done. I did not figure out how to add categories though. Can we somehow give project admins (see projects.kde.org) the required rights to that website? Or simply allow every KDE account to create a new subject and/or board? I just granted you admin access on todo.kde.org. Thanks. But, in general, shouldn't more people get rights on that page? Do we really need to request the rights to create a new board from the sysadmins? Same for categories? Unfortunately the software is quite inflexible; there's only a coarse-grained Administrator checkbox you can tick for users, which lets them do *everything* (including giving and removing administrator access from others!). Personally I'd be reluctant to give administrator access to everyone... If kanboard had more fine-grained ACLs, I agree that more people should have access to things like editing categories. -- Nicolás
Re: Re: Using Gerrit for code review in KDE
On Saturday 13 September 2014 20:38:21 Eike Hein wrote: The argument but you can still bypass Gerrit and push directly, this is just social etiquette doesn't matter because the whole thing is about social etiquette. The ACLs we already have reflect our social etiquette, and that means we need to be careful and think smart about evolving it. Personally I think social etiquette encouraging the use of review tools is fine. Social etiquette that entrenches some developers as special on those review tools is not. Thanks for raising your concerns! I think they are very valid and I think you touched a topic which we need to solve in more general: better handling of maintainer pass-over and removing ACL where it's not needed (repo ownership comes to my mind which is one of the areas I assume you meant). I also see that I should have elaborated more. I had written more and removed it before sending the mail. With review board I was quite often in the situation that I wanted to easily encode I like the presented solution, but do not know the code in question enough to give you a shipit. That's what a +1 is to me. On the other hand I was also in the situation that I wanted to know whether the review I got is from someone with knowledge to the code base. Currently a shipit means one as a developer has to do research whether the reviewer knows the code good enough. A +2 as I really know that code from a project core member would really help there. I have seen more than once that people not knowing the code good enough gave a shipit and it got pushed before people with the knowledge looked at it. I also had new developers in mind with my comment. Lets assume we have two new developers and the one giving the other a shipit and the other pushes but the code is in well say not yet good enough. If the maintainer reverts afterwards it might mean that we have lost a possible new contributor. I don't know whether this happens so often that we have to care about, but given that we move to pre-commit review I prefer not having to think about reverting at all. That said: I see a strong advantage in having the complete range from -2 to +2 available for development and -2,+2 only available to core members of the given repository. Giving +2 for everyone results in having the same as the shipit state currently: it's no easily interpretable way to read how familiar the reviewer is with the project. But you raised a good point. I think this needs more brainstorming and discussions on the community mailing list to find a good solution which keeps our core values. I think at the same time we should tackle the other already implemented ACLs. As long as that is not solved I agree that we shouldn't make too fast steps and not implement an ACL on gerrit. Cheers Martin signature.asc Description: This is a digitally signed message part.
Re: Using Gerrit for code review in KDE
On 13.09.2014 21:10, Kevin Krammer wrote: So your suggestion would be to not have +2 but a policy of some sort that only the +1 of the maintainer, if there is an active one, is considered as go ahead? Basically my thinking is roughly this: It actually happens extremely rarely in practice that something gets committed that needs to be reverted because it's actively objectiona- ble. As Ivan pointed out, few of us will ever commit any- thing if we're not confident it would meet with the approval of our peers. We generally do a good job with seeking out the opinion and approval of those in the know. Yes, this requires a lot of trust - but we've always known this; broad, equal write access requires just as much trust. That's why we have a process for getting developer access that tries to make sure those getting access have been around long enough to understand the etiquette and can be reasonably worked with; it's why getting developer access involves peer review by other developers. The idea is that trust can flow from this - when someone has a KDE developer account you want to be able to rely on the fact that they should have that account. Having a +2 and restricting it to maintainers says we can't trust KDE developer account holders. If this were actually true, I think it would imply our process is broken on the other end. The Manifesto is written in the spirit of rely- ing on this trust mechanic. But as said, I don't think it's actually true. I think we *can* trust KDE developer account holders, and that's why we don't need an extra safety net in the form of having a +2 and restricting it to maintainers. It's biasing the system in the wrong direction, and tries to plan for an eventuality that happens extremely rarely (and we have other safety nets: test suites, CI, beta releases, ...), at the cost of making maintainer succession more difficult down the road. Maintainers should always think about the maintainers who will one day replace them and make sure they can. If I brainstorm about alternatives I find: * let maintainers have -2 as pro-active variation of reverting That still requires flagging people as maintainers in a DB, though ... We already flag maintainers on projects.kde.org, which as mentioned I think was a mistake, but it mainly came about for logistical reasons, not for security reasons. I think we need to avoid using this as a precedent to entrench maintainers elsewhere ACL-wise. * build social ettiquette to always wait for the maintainer's +1 but at most for a certain grace period, e.g. one week I think this etiquette basically already exists in prac- tice. If I write a patch that touched complicated code in plasma- framework that I know Marco knows much better than I do, I'm not going to commit it without him having weighed in on it. If Marco's on vacation I'll at least make sure someone else thought through the problem and came to the same conclusion as I did. I fixed some bugs in KIO lately around file previews, and struck up an in-depth conversation with David about it and made sure I got his review and input for the changes I needed. And so on ... all without requiring a +2. * have everyone get +2 and use etiquette to do that only if there is already strong agreement or the grace period has passed Seems best to me, assuming we can't change Gerrit to remove the distinction entirely. Cheers, Kevin Cheers, Eike
Re: Administrating project boards on todo.kde.org
On Sun, Sep 14, 2014 at 7:20 AM, Nicolás Alvarez nicolas.alva...@gmail.com wrote: 2014-09-13 15:57 GMT-03:00 Milian Wolff m...@milianw.de: On Saturday 13 September 2014 14:38:03 Nicolás Alvarez wrote: 2014-09-13 14:25 GMT-03:00 Milian Wolff m...@milianw.de: Hey all, I hope this is the right place to ask. I would like to start using todo.kde.org more. It's imo a good place to track jobs that need to be done. I did not figure out how to add categories though. Can we somehow give project admins (see projects.kde.org) the required rights to that website? Or simply allow every KDE account to create a new subject and/or board? I just granted you admin access on todo.kde.org. Thanks. But, in general, shouldn't more people get rights on that page? Do we really need to request the rights to create a new board from the sysadmins? Same for categories? Unfortunately the software is quite inflexible; there's only a coarse-grained Administrator checkbox you can tick for users, which lets them do *everything* (including giving and removing administrator access from others!). Personally I'd be reluctant to give administrator access to everyone... If kanboard had more fine-grained ACLs, I agree that more people should have access to things like editing categories. At the moment our solution to this is to grant administrator access to a wide range of people in the community. Unfortunately it is an all or nothing approach. -- Nicolás Thanks, Ben
Re: Using Gerrit for code review in KDE
that needs to be reverted because it's actively objectiona- ble. As Ivan pointed out, few of us will ever commit any- thing if we're not confident it would meet with the approval While I do agree that we have a strange and unreally awesome community that behaves really well (and I do trust most KDE devs), I was approaching to this from the same angle as Martin. Namely, for the projects that I know the people who are actually the /core/ team, I always wait their input before pushing something. For those that I don't know, I need to check who is in charge, and whether a 'ship it' I got actually has any weight behind it. +2 would show a newcommer that the review is really by someone who (1) looked it in-detail, and (2) by someone who actually knows what he is talking about. (this might sound overly strict, but I guess you know what I meant by this) For me, it is not about trust. But rather about providing additional information to the submitter. That is why I don't think that the requirement for the 'submit' does not need to be limitted to the maintainers/core team. Also, Kevin's idea of +1s that got more weight over time (aka the inactive- core-team mode) seems nice, though I don't think 1 week is the right ammount of time. Cheerio, Ivan -- KDE, ivan.cukic at kde.org, http://ivan.fomentgroup.org/ gpg key id: 850B6F76, keyserver.pgp.com
Re: Using Gerrit for code review in KDE
On Sun, Sep 14, 2014 at 8:07 AM, Ivan Čukić ivan.cu...@kde.org wrote: that needs to be reverted because it's actively objectiona- ble. As Ivan pointed out, few of us will ever commit any- thing if we're not confident it would meet with the approval While I do agree that we have a strange and unreally awesome community that behaves really well (and I do trust most KDE devs), I was approaching to this from the same angle as Martin. Namely, for the projects that I know the people who are actually the /core/ team, I always wait their input before pushing something. For those that I don't know, I need to check who is in charge, and whether a 'ship it' I got actually has any weight behind it. +2 would show a newcommer that the review is really by someone who (1) looked it in-detail, and (2) by someone who actually knows what he is talking about. (this might sound overly strict, but I guess you know what I meant by this) Shouldn't this be up to the reviewer to use their good judgement when deciding whether to use +1 or +2? If they're not the maintainer or don't know the codebase well enough, then granting +2 would be rather unusual from a social point of view. For me, it is not about trust. But rather about providing additional information to the submitter. That is why I don't think that the requirement for the 'submit' does not need to be limitted to the maintainers/core team. Also, Kevin's idea of +1s that got more weight over time (aka the inactive- core-team mode) seems nice, though I don't think 1 week is the right ammount of time. Cheerio, Ivan Thanks, Ben -- KDE, ivan.cukic at kde.org, http://ivan.fomentgroup.org/ gpg key id: 850B6F76, keyserver.pgp.com
Re: Using Gerrit for code review in KDE
On Sunday 14 September 2014 08:11:43 Ben Cooksley wrote: On Sun, Sep 14, 2014 at 8:07 AM, Ivan Čukić ivan.cu...@kde.org wrote: that needs to be reverted because it's actively objectiona- ble. As Ivan pointed out, few of us will ever commit any- thing if we're not confident it would meet with the approval While I do agree that we have a strange and unreally awesome community that behaves really well (and I do trust most KDE devs), I was approaching to this from the same angle as Martin. Namely, for the projects that I know the people who are actually the /core/ team, I always wait their input before pushing something. For those that I don't know, I need to check who is in charge, and whether a 'ship it' I got actually has any weight behind it. +2 would show a newcommer that the review is really by someone who (1) looked it in-detail, and (2) by someone who actually knows what he is talking about. (this might sound overly strict, but I guess you know what I meant by this) Shouldn't this be up to the reviewer to use their good judgement when deciding whether to use +1 or +2? If they're not the maintainer or don't know the codebase well enough, then granting +2 would be rather unusual from a social point of view. I agree here. Everyone with a KDE developer account should in principle have the right to give a +2. One should only use it when appropriate though, i.e. when one is the maintainer of a given piece of code or when the patch is simple enough so that one feels safe to give the other the ship-it. In ReviewBoard it's the same currently, no? Bye -- Milian Wolff m...@milianw.de http://milianw.de
Re: Using Gerrit for code review in KDE
Everyone with a KDE developer account should in principle have the right to give a +2. One should only use it when appropriate though, i.e. when one is the maintainer of a given piece of code or when the patch is simple enough so that one feels safe to give the other the ship-it. That's my opinion as well. It would be nice to have an explicit way to differentiate the I think this patch is okay, but I'm not very familiar with the code you changed (+1) and I'm confident this patch is fine (+2) cases, and I think everyone with a KDE dev account is capable of deciding which one to select by himself when reviewing, without a technical restriction on what one can do. Greetings!
Re: Using Gerrit for code review in KDE
On 13.09.2014 22:50, Sven Brauch wrote: That's my opinion as well. It would be nice to have an explicit way to differentiate the I think this patch is okay, but I'm not very familiar with the code you changed (+1) and I'm confident this patch is fine (+2) cases, and I think everyone with a KDE dev account is capable of deciding which one to select by himself when reviewing, without a technical restriction on what one can do. Yeah, that's something I'm OK with too. Maybe we can even adapt the UI to use strings like Sven proposes? Greetings! Cheers, Eike
Re: Using Gerrit for code review in KDE
On 12 Sep 2014 22:53, Marco Martin notm...@gmail.com wrote: On Tuesday, September 9, 2014, Jan Kundrát j...@flaska.net wrote: If you would like all plasma to go, just give me a list of repos and I can make it happen. No, definitely not yet In my opinion, the purpose of this test is not to verify that Gerrit works or that the ACLs are set up properly -- both were done already. As part of the experiment i would also like to try to have stricter acls for +2 and submit, like starting from mantainers then slowly adding people (that's also how i understood it would have worked during the bof) My understanding from the BOF that it would be purely social, and we would add if we need it. It's already better than reviewboard given that we have that +1, +2 separation. I think a good example is your patch today (and pretending you're not a maintainer). There was a single typo in a commit message. I wanted it fixing, but I don't want to have to have to review that whole thing again (in reviewboard terms fix it and ship it). I would have given a +2, but when you re-push to gerrit I would have to +2 again before you can merge. It's be a perfect example of where a self +2 would be fine. David
Re: Using Gerrit for code review in KDE
On Saturday 13 September 2014 23:29:55 David Edmundson wrote: On 12 Sep 2014 22:53, Marco Martin notm...@gmail.com wrote: On Tuesday, September 9, 2014, Jan Kundrát j...@flaska.net wrote: If you would like all plasma to go, just give me a list of repos and I can make it happen. No, definitely not yet In my opinion, the purpose of this test is not to verify that Gerrit works or that the ACLs are set up properly -- both were done already. As part of the experiment i would also like to try to have stricter acls for +2 and submit, like starting from mantainers then slowly adding people (that's also how i understood it would have worked during the bof) My understanding from the BOF that it would be purely social, and we would add if we need it. It's already better than reviewboard given that we have that +1, +2 separation. I think a good example is your patch today (and pretending you're not a maintainer). There was a single typo in a commit message. I wanted it fixing, but I don't want to have to have to review that whole thing again (in reviewboard terms fix it and ship it). I would have given a +2, but when you re-push to gerrit I would have to +2 again before you can merge. It's be a perfect example of where a self +2 would be fine. This, btw, is accepted behavior in Qt for those that are approvers or maintainers. In KDE, everyone has that status. So everyone can +2 himself if needed. Bye -- Milian Wolff m...@milianw.de http://milianw.de
