Re: lack of reviewers
On 23 May 2012 06:34, Peter Hutterer wrote: > On 18/05/12 21:17 , Michal Suchanek wrote: >> >> On 18 May 2012 12:40, Peter Hutterer wrote: >>> >>> On 18/05/12 19:26 , Michal Suchanek wrote: On 18 May 2012 01:14, Peter Hutterer wrote: > > > On Thu, May 17, 2012 at 10:39:55AM +0200, Ernst Sjöstrand wrote: >> >> >> Hi, >> >> (sorry for jumping in from the outside and breaking the thread!) >> >> I read about this problem and wanted to offer a suggestion! >> >> What if you set up a Gerrit server for git.freedesktop.org? That's the >> tool the Android OpenSource project uses among other things: >> https://android-review.googlesource.com/ >> Perhaps if it was easier to contribute to reviewing code, more people >> would do it more often? > > > >> It's also a very nice tool I have to say, I use it every day at work. >> It's easy to integrate with automatic >> testing of patchsets before they're submitted to the repository for >> example. > > > > tbh I doubt what we have is a tool problem. Patches are sent to the > list > and > can be reviewed quite easily there (for subscribers, anyway). The issue > we > have is manpower and, more importantly, manpower of people with enough > knowledge to judge whether a patchset has side-effects beyond the > obvious. > > in the end, such patches tend fall on the shoulders of a few and adding > another tool that they have to check will increase, not decrease, the > workload for those. tbh using a mailing list for that looks very impractical. - patches get missed completely >>> >>> >>> >>> true. we do encourage people to re-submit. which, aside from the obvious >>> disadvantage, has advantages too. I found the problem with any todo list >>> is >>> that sooner or later it becomes so long that you either have to wipe it >>> (by >>> switching to a different system sometimes) or you start ignoring stuff >>> anyway. >>> >>> given that one of the problems is reviewer bandwidth, I expect this to >>> happen with any new tool. patchwork was great in the first few weeks, now >>> it's a kitchensink great for getting URLs and not much else. >> >> >> Given that reviewer bandwidth is scarce it would be perhaps helpful to >> spare it by having a tool that presents all the not-yet-reviewed >> patches in one place rather than reviewers fishing for them in the >> mailing list or in the patchwork. > > > I believe that patchwork was supposed to be that tool, but either it's not > set up correctly or it just doesn't do that. I don't know but if patchwork > can do this, then this would be a simple immediate step. > > having said that - it can't be perfect without manual intervention - when > patches go through multiple revisions someone still has to mark them > manually as obsolete. Well, gerrit is a BTS just like bugzilla except it it branch tracking system, not bug tracking system. So it likely has states for new, in-review, needs-update, obsolete, ... > > At several times in the past we noted the need for someone to just collect > leftover patches but that never happened, usually due to lack of > time. we really need someone to actually do it. That's something that a tracking system is supposed to do, be it patchwork or gerrit. > > - there is no track of what is related to what (as in the part of the same patchset or new revision of the same patchset) >>> >>> >>> >>> patch are usually in numbered series, in threads, with new revisinos >>> being >>> prefixed with "v2", "v3", etc. requires submitter discipline but it works >>> to >>> some degree. >> >> >> And as some of the patches get replies they get out of order and >> completely out of context. >> >>> >>> - you get a lots of list noise due to patches being sent one by one >>> >>> >>> >>> I'm not sure I follow this argument, can you expand? >> >> >> Like a series of 10+ patches for some part of the X server I do not >> understand landing on the list several times. >> >> I guess some people are fond of replying to the patches and quoting >> them in their email client and I can see that as nice feature but it's >> paid for by tons of list traffic. Necessarily large part of that is >> meaningless to most. >> >> The alternative is, of course, a link to git branch or something like >> that. > > > git branches are _incredibly_ painful to review. they're good for testing, > but the steps involved to point out an issue in a specific hunk of a > specific commit involves a lot more steps than just hitting reply and > starting to type. > Gerrit has an interface for that. Not sure how well it works since I never used gerrit extensively but it's obviously doable. Thanks Michal ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mai
Re: lack of reviewers
On 18/05/12 21:17 , Michal Suchanek wrote: On 18 May 2012 12:40, Peter Hutterer wrote: On 18/05/12 19:26 , Michal Suchanek wrote: On 18 May 2012 01:14, Peter Huttererwrote: On Thu, May 17, 2012 at 10:39:55AM +0200, Ernst Sjöstrand wrote: Hi, (sorry for jumping in from the outside and breaking the thread!) I read about this problem and wanted to offer a suggestion! What if you set up a Gerrit server for git.freedesktop.org? That's the tool the Android OpenSource project uses among other things: https://android-review.googlesource.com/ Perhaps if it was easier to contribute to reviewing code, more people would do it more often? It's also a very nice tool I have to say, I use it every day at work. It's easy to integrate with automatic testing of patchsets before they're submitted to the repository for example. tbh I doubt what we have is a tool problem. Patches are sent to the list and can be reviewed quite easily there (for subscribers, anyway). The issue we have is manpower and, more importantly, manpower of people with enough knowledge to judge whether a patchset has side-effects beyond the obvious. in the end, such patches tend fall on the shoulders of a few and adding another tool that they have to check will increase, not decrease, the workload for those. tbh using a mailing list for that looks very impractical. - patches get missed completely true. we do encourage people to re-submit. which, aside from the obvious disadvantage, has advantages too. I found the problem with any todo list is that sooner or later it becomes so long that you either have to wipe it (by switching to a different system sometimes) or you start ignoring stuff anyway. given that one of the problems is reviewer bandwidth, I expect this to happen with any new tool. patchwork was great in the first few weeks, now it's a kitchensink great for getting URLs and not much else. Given that reviewer bandwidth is scarce it would be perhaps helpful to spare it by having a tool that presents all the not-yet-reviewed patches in one place rather than reviewers fishing for them in the mailing list or in the patchwork. I believe that patchwork was supposed to be that tool, but either it's not set up correctly or it just doesn't do that. I don't know but if patchwork can do this, then this would be a simple immediate step. having said that - it can't be perfect without manual intervention - when patches go through multiple revisions someone still has to mark them manually as obsolete. requiring people to ping when patches get missed at least notifies us which patches have people behind them that care. a feature that gets submitted once, forgotten and no-one pings for it may not have been that important to begin with. When you get the 5th patch for the same regression submitted the third time it starts to look like shouting your patches off a cliff in the dead of the night. Usually it helps directly CC-ing the maintainer of that subsystem when the patch is ignored the first time. note - this only works because we don't get CCd on every patch, so the signal/noise ratio is still reasonably good. (Also, I say "we", the above is true for me so I just assume it's similar for others :) At several times in the past we noted the need for someone to just collect leftover patches but that never happened, usually due to lack of time. we really need someone to actually do it. - there is no track of what is related to what (as in the part of the same patchset or new revision of the same patchset) patch are usually in numbered series, in threads, with new revisinos being prefixed with "v2", "v3", etc. requires submitter discipline but it works to some degree. And as some of the patches get replies they get out of order and completely out of context. - you get a lots of list noise due to patches being sent one by one I'm not sure I follow this argument, can you expand? Like a series of 10+ patches for some part of the X server I do not understand landing on the list several times. I guess some people are fond of replying to the patches and quoting them in their email client and I can see that as nice feature but it's paid for by tons of list traffic. Necessarily large part of that is meaningless to most. The alternative is, of course, a link to git branch or something like that. git branches are _incredibly_ painful to review. they're good for testing, but the steps involved to point out an issue in a specific hunk of a specific commit involves a lot more steps than just hitting reply and starting to type. Cheers, Peter ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
On 05/20/2012 08:42 AM, Simon Thum wrote: > Hi all, > > I would like to tell another story, perhaps you follow me. > > I work on a project with a quite sharp gradient of competence in the > development team. We do use gerrit and a build server which verifies our > patches. > > The effect of gerrit - in our case! - is to encourage the less > knowledgeable people to do more complicated changes because they can > rely on the infrastructure to catch most errors for them. Also, we all > sleep better knowing it's hard to break the thing. > > Gerrit discerns "review" from humans from "validation" e.g. from > tinderbox. Every patch is validated independently, as far as is > possible. You know pretty fast when and where you made a mistake. Also > it will email you (configurable at the user level!) so you don't need to > check it actively if you don't like. > > For us it pays. But it also costs: > > The build process and unit testing has to be handled more rigid. We have > a lot of surrounding infrastructure just to nail the builds and tests. > It regularly needs attention to maintain. Unfortunately, the X server really has near-zero test coverage. I would not rely on compilation and existing test coverage as confirmation that there will likely not be regressions in xserver. Not yet at least. > All in all, I think this not really a social problem - it's a social > problem with technological roots, meaning it can be relieved > substantially. But it's not likely to be easy even if the tinderbox > already exists. I think the larger issue to potentially resolving this issue is for someone to do something about it. If someone who believes in gerrit sets up an instance and goes through the trouble to make it work, then I and hopefully others would be glad to try it out. X.org doesn't have funded leadership that can tell people to do specific things, like create a gerrit instance. It is up to individual developers to make the effort. -- Chase ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
Hi all, I would like to tell another story, perhaps you follow me. I work on a project with a quite sharp gradient of competence in the development team. We do use gerrit and a build server which verifies our patches. The effect of gerrit - in our case! - is to encourage the less knowledgeable people to do more complicated changes because they can rely on the infrastructure to catch most errors for them. Also, we all sleep better knowing it's hard to break the thing. Gerrit discerns "review" from humans from "validation" e.g. from tinderbox. Every patch is validated independently, as far as is possible. You know pretty fast when and where you made a mistake. Also it will email you (configurable at the user level!) so you don't need to check it actively if you don't like. For us it pays. But it also costs: The build process and unit testing has to be handled more rigid. We have a lot of surrounding infrastructure just to nail the builds and tests. It regularly needs attention to maintain. All in all, I think this not really a social problem - it's a social problem with technological roots, meaning it can be relieved substantially. But it's not likely to be easy even if the tinderbox already exists. HTH, Simon On 05/18/2012 01:14 AM, Peter Hutterer wrote: On Thu, May 17, 2012 at 10:39:55AM +0200, Ernst Sjöstrand wrote: Hi, (sorry for jumping in from the outside and breaking the thread!) I read about this problem and wanted to offer a suggestion! What if you set up a Gerrit server for git.freedesktop.org? That's the tool the Android OpenSource project uses among other things: https://android-review.googlesource.com/ Perhaps if it was easier to contribute to reviewing code, more people would do it more often? It's also a very nice tool I have to say, I use it every day at work. It's easy to integrate with automatic testing of patchsets before they're submitted to the repository for example. tbh I doubt what we have is a tool problem. Patches are sent to the list and can be reviewed quite easily there (for subscribers, anyway). The issue we have is manpower and, more importantly, manpower of people with enough knowledge to judge whether a patchset has side-effects beyond the obvious. in the end, such patches tend fall on the shoulders of a few and adding another tool that they have to check will increase, not decrease, the workload for those. Cheers, Peter ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
Hi, On 18 May 2012 20:05, Maarten Maathuis wrote: > On Fri, May 18, 2012 at 5:56 PM, Michal Suchanek wrote: >> Of course it interferes with the existing process. >> >> If it was not used it would be of no use. > > You don't have to make it required, if the system is successful people > will migrate out of free will. I'm not sure I have too much to add to this discussion, since I'm yet another person who's entirely qualified to do reviews but just isn't. As for the tool issues, however - patchwork exists and does that, and does indeed have a git hook that's supposed to clean up patches which have been committed. Whether or not this works or happens is irrelevant, the fact is that it could be made to do so if it doesn't already with much less typing than has been expended on this thread already. Cheers, Daniel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
On Fri, May 18, 2012 at 5:56 PM, Michal Suchanek wrote: > On 18 May 2012 17:38, Maarten Maathuis wrote: > >> >> I think that a tool that allows you to see diffs in a web interface >> and do point-and-click defect submission would never hurt. As long as >> it doesn't interfere with existing processes (too much). > > Of course it interferes with the existing process. > > If it was not used it would be of no use. > > Thanks > > Michal You don't have to make it required, if the system is successful people will migrate out of free will. -- Far away from the primal instinct, the song seems to fade away, the river get wider between your thoughts and the things we do and say. ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
On 18 May 2012 17:38, Maarten Maathuis wrote: > > I think that a tool that allows you to see diffs in a web interface > and do point-and-click defect submission would never hurt. As long as > it doesn't interfere with existing processes (too much). Of course it interferes with the existing process. If it was not used it would be of no use. Thanks Michal ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
On Fri, May 18, 2012 at 1:17 PM, Michal Suchanek wrote: > On 18 May 2012 12:40, Peter Hutterer wrote: >> On 18/05/12 19:26 , Michal Suchanek wrote: >>> >>> On 18 May 2012 01:14, Peter Hutterer wrote: On Thu, May 17, 2012 at 10:39:55AM +0200, Ernst Sjöstrand wrote: > > Hi, > > (sorry for jumping in from the outside and breaking the thread!) > > I read about this problem and wanted to offer a suggestion! > > What if you set up a Gerrit server for git.freedesktop.org? That's the > tool the Android OpenSource project uses among other things: > https://android-review.googlesource.com/ > Perhaps if it was easier to contribute to reviewing code, more people > would do it more often? > It's also a very nice tool I have to say, I use it every day at work. > It's easy to integrate with automatic > testing of patchsets before they're submitted to the repository for > example. tbh I doubt what we have is a tool problem. Patches are sent to the list and can be reviewed quite easily there (for subscribers, anyway). The issue we have is manpower and, more importantly, manpower of people with enough knowledge to judge whether a patchset has side-effects beyond the obvious. in the end, such patches tend fall on the shoulders of a few and adding another tool that they have to check will increase, not decrease, the workload for those. >>> >>> >>> tbh using a mailing list for that looks very impractical. >>> >>> - patches get missed completely >> >> >> true. we do encourage people to re-submit. which, aside from the obvious >> disadvantage, has advantages too. I found the problem with any todo list is >> that sooner or later it becomes so long that you either have to wipe it (by >> switching to a different system sometimes) or you start ignoring stuff >> anyway. >> >> given that one of the problems is reviewer bandwidth, I expect this to >> happen with any new tool. patchwork was great in the first few weeks, now >> it's a kitchensink great for getting URLs and not much else. > > Given that reviewer bandwidth is scarce it would be perhaps helpful to > spare it by having a tool that presents all the not-yet-reviewed > patches in one place rather than reviewers fishing for them in the > mailing list or in the patchwork. > >> >> requiring people to ping when patches get missed at least notifies us which >> patches have people behind them that care. a feature that gets submitted >> once, forgotten and no-one pings for it may not have been that important to >> begin with. > > When you get the 5th patch for the same regression submitted the third > time it starts to look like shouting your patches off a cliff in the > dead of the night. > > >> >> >>> - there is no track of what is related to what (as in the part of the >>> same patchset or new revision of the same patchset) >> >> >> patch are usually in numbered series, in threads, with new revisinos being >> prefixed with "v2", "v3", etc. requires submitter discipline but it works to >> some degree. > > And as some of the patches get replies they get out of order and > completely out of context. > >> >> >>> - you get a lots of list noise due to patches being sent one by one >> >> >> I'm not sure I follow this argument, can you expand? > > Like a series of 10+ patches for some part of the X server I do not > understand landing on the list several times. > > I guess some people are fond of replying to the patches and quoting > them in their email client and I can see that as nice feature but it's > paid for by tons of list traffic. Necessarily large part of that is > meaningless to most. > > The alternative is, of course, a link to git branch or something like that. > > Thanks > > Michal > ___ > xorg-devel@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel I think that a tool that allows you to see diffs in a web interface and do point-and-click defect submission would never hurt. As long as it doesn't interfere with existing processes (too much). -- Far away from the primal instinct, the song seems to fade away, the river get wider between your thoughts and the things we do and say. ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
On 18 May 2012 12:40, Peter Hutterer wrote: > On 18/05/12 19:26 , Michal Suchanek wrote: >> >> On 18 May 2012 01:14, Peter Hutterer wrote: >>> >>> On Thu, May 17, 2012 at 10:39:55AM +0200, Ernst Sjöstrand wrote: Hi, (sorry for jumping in from the outside and breaking the thread!) I read about this problem and wanted to offer a suggestion! What if you set up a Gerrit server for git.freedesktop.org? That's the tool the Android OpenSource project uses among other things: https://android-review.googlesource.com/ Perhaps if it was easier to contribute to reviewing code, more people would do it more often? >>> >>> It's also a very nice tool I have to say, I use it every day at work. It's easy to integrate with automatic testing of patchsets before they're submitted to the repository for example. >>> >>> >>> tbh I doubt what we have is a tool problem. Patches are sent to the list >>> and >>> can be reviewed quite easily there (for subscribers, anyway). The issue >>> we >>> have is manpower and, more importantly, manpower of people with enough >>> knowledge to judge whether a patchset has side-effects beyond the >>> obvious. >>> >>> in the end, such patches tend fall on the shoulders of a few and adding >>> another tool that they have to check will increase, not decrease, the >>> workload for those. >> >> >> tbh using a mailing list for that looks very impractical. >> >> - patches get missed completely > > > true. we do encourage people to re-submit. which, aside from the obvious > disadvantage, has advantages too. I found the problem with any todo list is > that sooner or later it becomes so long that you either have to wipe it (by > switching to a different system sometimes) or you start ignoring stuff > anyway. > > given that one of the problems is reviewer bandwidth, I expect this to > happen with any new tool. patchwork was great in the first few weeks, now > it's a kitchensink great for getting URLs and not much else. Given that reviewer bandwidth is scarce it would be perhaps helpful to spare it by having a tool that presents all the not-yet-reviewed patches in one place rather than reviewers fishing for them in the mailing list or in the patchwork. > > requiring people to ping when patches get missed at least notifies us which > patches have people behind them that care. a feature that gets submitted > once, forgotten and no-one pings for it may not have been that important to > begin with. When you get the 5th patch for the same regression submitted the third time it starts to look like shouting your patches off a cliff in the dead of the night. > > >> - there is no track of what is related to what (as in the part of the >> same patchset or new revision of the same patchset) > > > patch are usually in numbered series, in threads, with new revisinos being > prefixed with "v2", "v3", etc. requires submitter discipline but it works to > some degree. And as some of the patches get replies they get out of order and completely out of context. > > >> - you get a lots of list noise due to patches being sent one by one > > > I'm not sure I follow this argument, can you expand? Like a series of 10+ patches for some part of the X server I do not understand landing on the list several times. I guess some people are fond of replying to the patches and quoting them in their email client and I can see that as nice feature but it's paid for by tons of list traffic. Necessarily large part of that is meaningless to most. The alternative is, of course, a link to git branch or something like that. Thanks Michal ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
On 18/05/12 17:43 , walter harms wrote: Am 18.05.2012 01:14, schrieb Peter Hutterer: On Thu, May 17, 2012 at 10:39:55AM +0200, Ernst Sjöstrand wrote: Hi, (sorry for jumping in from the outside and breaking the thread!) I read about this problem and wanted to offer a suggestion! What if you set up a Gerrit server for git.freedesktop.org? That's the tool the Android OpenSource project uses among other things: https://android-review.googlesource.com/ Perhaps if it was easier to contribute to reviewing code, more people would do it more often? It's also a very nice tool I have to say, I use it every day at work. It's easy to integrate with automatic testing of patchsets before they're submitted to the repository for example. tbh I doubt what we have is a tool problem. Patches are sent to the list and can be reviewed quite easily there (for subscribers, anyway). The issue we have is manpower and, more importantly, manpower of people with enough knowledge to judge whether a patchset has side-effects beyond the obvious. in the end, such patches tend fall on the shoulders of a few and adding another tool that they have to check will increase, not decrease, the workload for those. Maybe i can be useful, since i am not a core developer but i review patches from time to time on the code only. For me the biggest problem is that i can not get the whole picture easly. each patch has two components - the code and the big picture. there are plenty of patches where the big picture is easy enough (if you know it) but looking at the code is tedious. so a rev-by for the code correctness allows others to focus on the big picture stuff. and sooner or later, you'll start seeing the big picture too. so don't underestimate "just code" reviews, they're very helpful. Cheers, Peter > When i do the same with linux patches i can go to LXR and get an idea what is going on. For *me* it would be helpful to have such a beast for X11. Getting more people into reviewing is a hard business because the motivation can be very different. just my 2 cents, re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
On 18/05/12 19:26 , Michal Suchanek wrote: On 18 May 2012 01:14, Peter Hutterer wrote: On Thu, May 17, 2012 at 10:39:55AM +0200, Ernst Sjöstrand wrote: Hi, (sorry for jumping in from the outside and breaking the thread!) I read about this problem and wanted to offer a suggestion! What if you set up a Gerrit server for git.freedesktop.org? That's the tool the Android OpenSource project uses among other things: https://android-review.googlesource.com/ Perhaps if it was easier to contribute to reviewing code, more people would do it more often? It's also a very nice tool I have to say, I use it every day at work. It's easy to integrate with automatic testing of patchsets before they're submitted to the repository for example. tbh I doubt what we have is a tool problem. Patches are sent to the list and can be reviewed quite easily there (for subscribers, anyway). The issue we have is manpower and, more importantly, manpower of people with enough knowledge to judge whether a patchset has side-effects beyond the obvious. in the end, such patches tend fall on the shoulders of a few and adding another tool that they have to check will increase, not decrease, the workload for those. tbh using a mailing list for that looks very impractical. - patches get missed completely true. we do encourage people to re-submit. which, aside from the obvious disadvantage, has advantages too. I found the problem with any todo list is that sooner or later it becomes so long that you either have to wipe it (by switching to a different system sometimes) or you start ignoring stuff anyway. given that one of the problems is reviewer bandwidth, I expect this to happen with any new tool. patchwork was great in the first few weeks, now it's a kitchensink great for getting URLs and not much else. requiring people to ping when patches get missed at least notifies us which patches have people behind them that care. a feature that gets submitted once, forgotten and no-one pings for it may not have been that important to begin with. - there is no track of what is related to what (as in the part of the same patchset or new revision of the same patchset) patch are usually in numbered series, in threads, with new revisinos being prefixed with "v2", "v3", etc. requires submitter discipline but it works to some degree. - you get a lots of list noise due to patches being sent one by one I'm not sure I follow this argument, can you expand? don't get me wrong, I don't think mailing lists is the ideal approach but I haven't seen a better one. it is an old approach and many people are well set up for the workflow so any new tool will have to have meet quite a threshold there. Cheers, Peter ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
On 18 May 2012 01:14, Peter Hutterer wrote: > On Thu, May 17, 2012 at 10:39:55AM +0200, Ernst Sjöstrand wrote: >> Hi, >> >> (sorry for jumping in from the outside and breaking the thread!) >> >> I read about this problem and wanted to offer a suggestion! >> >> What if you set up a Gerrit server for git.freedesktop.org? That's the >> tool the Android OpenSource project uses among other things: >> https://android-review.googlesource.com/ >> Perhaps if it was easier to contribute to reviewing code, more people >> would do it more often? > >> It's also a very nice tool I have to say, I use it every day at work. >> It's easy to integrate with automatic >> testing of patchsets before they're submitted to the repository for example. > > tbh I doubt what we have is a tool problem. Patches are sent to the list and > can be reviewed quite easily there (for subscribers, anyway). The issue we > have is manpower and, more importantly, manpower of people with enough > knowledge to judge whether a patchset has side-effects beyond the obvious. > > in the end, such patches tend fall on the shoulders of a few and adding > another tool that they have to check will increase, not decrease, the > workload for those. tbh using a mailing list for that looks very impractical. - patches get missed completely - there is no track of what is related to what (as in the part of the same patchset or new revision of the same patchset) - you get a lots of list noise due to patches being sent one by one Thanks Michal ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
On 18 May 2012 01:15, Peter Hutterer wrote: > On Thu, May 17, 2012 at 01:15:25PM +0200, Michal Suchanek wrote: >> On 17 May 2012 10:56, Olivier Galibert wrote: >> > On Thu, May 17, 2012 at 10:39:55AM +0200, Ernst Sjöstrand wrote: >> >> What if you set up a Gerrit server for git.freedesktop.org? >> > >> > When you have a social problem and try to handle it with technology, >> > you end up with two problems. There has been no specific grumblings >> > against the review methodology. >> > >> >> It could, however, solve the problem with patchwork not noting which >> patches are applied. > > shouldn't that be fixed in patchwork instead of introducing another tool? > It depends. Sometimes replacing the broken wheel is easier than fixing it. Is anybody working on fixing patchwork? Thanks Michal ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
Am 18.05.2012 01:14, schrieb Peter Hutterer: > On Thu, May 17, 2012 at 10:39:55AM +0200, Ernst Sjöstrand wrote: >> Hi, >> >> (sorry for jumping in from the outside and breaking the thread!) >> >> I read about this problem and wanted to offer a suggestion! >> >> What if you set up a Gerrit server for git.freedesktop.org? That's the >> tool the Android OpenSource project uses among other things: >> https://android-review.googlesource.com/ >> Perhaps if it was easier to contribute to reviewing code, more people >> would do it more often? > >> It's also a very nice tool I have to say, I use it every day at work. >> It's easy to integrate with automatic >> testing of patchsets before they're submitted to the repository for example. > > tbh I doubt what we have is a tool problem. Patches are sent to the list and > can be reviewed quite easily there (for subscribers, anyway). The issue we > have is manpower and, more importantly, manpower of people with enough > knowledge to judge whether a patchset has side-effects beyond the obvious. > > in the end, such patches tend fall on the shoulders of a few and adding > another tool that they have to check will increase, not decrease, the > workload for those. > Maybe i can be useful, since i am not a core developer but i review patches from time to time on the code only. For me the biggest problem is that i can not get the whole picture easly. When i do the same with linux patches i can go to LXR and get an idea what is going on. For *me* it would be helpful to have such a beast for X11. Getting more people into reviewing is a hard business because the motivation can be very different. just my 2 cents, re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
On Thu, May 17, 2012 at 01:15:25PM +0200, Michal Suchanek wrote: > On 17 May 2012 10:56, Olivier Galibert wrote: > > On Thu, May 17, 2012 at 10:39:55AM +0200, Ernst Sjöstrand wrote: > >> What if you set up a Gerrit server for git.freedesktop.org? > > > > When you have a social problem and try to handle it with technology, > > you end up with two problems. There has been no specific grumblings > > against the review methodology. > > > > It could, however, solve the problem with patchwork not noting which > patches are applied. shouldn't that be fixed in patchwork instead of introducing another tool? Cheers, Peter ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
On Thu, May 17, 2012 at 10:39:55AM +0200, Ernst Sjöstrand wrote: > Hi, > > (sorry for jumping in from the outside and breaking the thread!) > > I read about this problem and wanted to offer a suggestion! > > What if you set up a Gerrit server for git.freedesktop.org? That's the > tool the Android OpenSource project uses among other things: > https://android-review.googlesource.com/ > Perhaps if it was easier to contribute to reviewing code, more people > would do it more often? > It's also a very nice tool I have to say, I use it every day at work. > It's easy to integrate with automatic > testing of patchsets before they're submitted to the repository for example. tbh I doubt what we have is a tool problem. Patches are sent to the list and can be reviewed quite easily there (for subscribers, anyway). The issue we have is manpower and, more importantly, manpower of people with enough knowledge to judge whether a patchset has side-effects beyond the obvious. in the end, such patches tend fall on the shoulders of a few and adding another tool that they have to check will increase, not decrease, the workload for those. Cheers, Peter ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
On 17 May 2012 10:56, Olivier Galibert wrote: > On Thu, May 17, 2012 at 10:39:55AM +0200, Ernst Sjöstrand wrote: >> What if you set up a Gerrit server for git.freedesktop.org? > > When you have a social problem and try to handle it with technology, > you end up with two problems. There has been no specific grumblings > against the review methodology. > It could, however, solve the problem with patchwork not noting which patches are applied. Thanks Michal ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
On Thu, May 17, 2012 at 10:39:55AM +0200, Ernst Sjöstrand wrote: > What if you set up a Gerrit server for git.freedesktop.org? When you have a social problem and try to handle it with technology, you end up with two problems. There has been no specific grumblings against the review methodology. Part of the review problem is due to the Dunning-Kruger effect. People reaching the "competent enough" point to do pertinent reviews don't think they're at that point. And part stems from technical difficulties regarding building and testing Mesa, which a whatever server won't solve. And a last part is that, as far as I can tell, the main reviewers are paid to do what they do, and nowadays that means spending way more time in meetings, reports and other fluff than anything they consider actual work. So when they have time for actual work, they'd rather spend their time in their code and their project than other people's. Best, OG. ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers
Hi, (sorry for jumping in from the outside and breaking the thread!) I read about this problem and wanted to offer a suggestion! What if you set up a Gerrit server for git.freedesktop.org? That's the tool the Android OpenSource project uses among other things: https://android-review.googlesource.com/ Perhaps if it was easier to contribute to reviewing code, more people would do it more often? It's also a very nice tool I have to say, I use it every day at work. It's easy to integrate with automatic testing of patchsets before they're submitted to the repository for example. Here's the site: http://code.google.com/p/gerrit/ And here's the documentation: http://gerrit-documentation.googlecode.com/svn/Documentation/2.3/index.html Regards //Ernst Sjöstrand ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers (Re: first set of new APIs + convert server to use them)
On Mon, May 14, 2012 at 07:22:48PM +0100, Dave Airlie wrote: > On Mon, May 14, 2012 at 7:04 PM, Alan Coopersmith > wrote: > > On 05/14/12 04:28 AM, Dave Airlie wrote: > >> nobody? ping? > >> > >> Like Keith the glyph picture API patch is like 2 lines long :) > >> > >> /me wonders how I'm going to get the next 50 patches in at this rate > >> some time this year. > > > > It's not just you, we seem to be having a harder time getting reviews done > > all around. > > > > (I admit I'm as guilty of this as the rest, since I've been busy on other > > things, and less motivated to review for other people when I can't get > > my patches reviewed.) > > > > I've got no ideas how to fix this quickly, but we need to get it fixed. > > > > Patchwork only helps if we get the finished reviews cleared off - right now > > everything is listed as needing review, even if we've integrated it, > > reworked it, or rejected it. > > I've pretty much no idea how to deal with it sanely. We've moved to larger > scale development model without a larger set of developers. The kernel > isn't even as stringent wrt to reviews as xorg-devel is. then we fix the process? it's not like it's written in stone. IMO having at least one more person in the loop to build before pushing is already quite useful to catch issues like missing git adds or spurious API breaks. one reason the current review process is in place is because before 1.8, the server was rather unpredictable and even getting one that built was a bit of luck. This has changed, and all patches at least show up on the list now. I'm pretty sure I've gotten comments about potential bugs from reviewers that otherwise wouldn't have added a rev-by. Another thing is that feature-branch development didn't really exist before 1.8. I think that has changed now as well. so I think loosening the review requirements is not the worst of all ideas, though it shouldn't be seen as a free-for-all. Cheers, Peter > I'm guessing we'll probably have to have review swap parties or meetings > or something insane like that to clear the backlog on occasions, > it would be nice if patchwork could be kept up to date, but it would involve > anyone handling patches to jump on and clean up the ones they've merged > already. > > I spent a major amount of my time either reviewing kernel patches, or > persuading others to review other peoples patches so I don't have to, > I'm not sure if we need some more tracking from Keith or others on > what unreviewed stuff is outstanding and who best to direct it to, but > again it involves a time commitment from someone and I've no idea > who could afford it. > > Like the input guys have a bit of crossover work, the build system > stuff seems to be covered, but the rest of the server is a wasteland > of unreviewedness. ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers (Re: first set of new APIs + convert server to use them)
On Mon, 14 May 2012 19:22:48 +0100, Dave Airlie wrote: > I've pretty much no idea how to deal with it sanely. We've moved to larger > scale development model without a larger set of developers. The kernel > isn't even as stringent wrt to reviews as xorg-devel is. Small changes seem to get reviewed pretty quickly; it's big changes that take actual thought that just aren't getting looked at quickly (if at all). > I'm guessing we'll probably have to have review swap parties or meetings > or something insane like that to clear the backlog on occasions, > it would be nice if patchwork could be kept up to date, but it would involve > anyone handling patches to jump on and clean up the ones they've merged > already. If patchwork could figure out when stuff got merged automatically, it would be a whole lot more useful. As it is, you have no idea which patches need review, which patches are waiting for me to pull them into the server and which patches are actually on master. > I spent a major amount of my time either reviewing kernel patches, or > persuading others to review other peoples patches so I don't have to, > I'm not sure if we need some more tracking from Keith or others on > what unreviewed stuff is outstanding and who best to direct it to, but > again it involves a time commitment from someone and I've no idea > who could afford it. I had a notmuch rule to find unreviewed patches; that made finding them easier at least; would it be useful to post the output of that on a regular basis? I'm pretty sure I could script it. > Like the input guys have a bit of crossover work, the build system > stuff seems to be covered, but the rest of the server is a wasteland > of unreviewedness. Yeah, always easier to get review on code near the edges than DIX stuff. No easy way to fix that unless we manage to get more people needing to fix DIX code... -- keith.pack...@intel.com pgpyrzVc0yHUP.pgp Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: lack of reviewers (Re: first set of new APIs + convert server to use them)
On Mon, May 14, 2012 at 7:04 PM, Alan Coopersmith wrote: > On 05/14/12 04:28 AM, Dave Airlie wrote: >> nobody? ping? >> >> Like Keith the glyph picture API patch is like 2 lines long :) >> >> /me wonders how I'm going to get the next 50 patches in at this rate >> some time this year. > > It's not just you, we seem to be having a harder time getting reviews done > all around. > > (I admit I'm as guilty of this as the rest, since I've been busy on other > things, and less motivated to review for other people when I can't get > my patches reviewed.) > > I've got no ideas how to fix this quickly, but we need to get it fixed. > > Patchwork only helps if we get the finished reviews cleared off - right now > everything is listed as needing review, even if we've integrated it, > reworked it, or rejected it. I've pretty much no idea how to deal with it sanely. We've moved to larger scale development model without a larger set of developers. The kernel isn't even as stringent wrt to reviews as xorg-devel is. I'm guessing we'll probably have to have review swap parties or meetings or something insane like that to clear the backlog on occasions, it would be nice if patchwork could be kept up to date, but it would involve anyone handling patches to jump on and clean up the ones they've merged already. I spent a major amount of my time either reviewing kernel patches, or persuading others to review other peoples patches so I don't have to, I'm not sure if we need some more tracking from Keith or others on what unreviewed stuff is outstanding and who best to direct it to, but again it involves a time commitment from someone and I've no idea who could afford it. Like the input guys have a bit of crossover work, the build system stuff seems to be covered, but the rest of the server is a wasteland of unreviewedness. Dave. ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
lack of reviewers (Re: first set of new APIs + convert server to use them)
On 05/14/12 04:28 AM, Dave Airlie wrote: > nobody? ping? > > Like Keith the glyph picture API patch is like 2 lines long :) > > /me wonders how I'm going to get the next 50 patches in at this rate > some time this year. It's not just you, we seem to be having a harder time getting reviews done all around. (I admit I'm as guilty of this as the rest, since I've been busy on other things, and less motivated to review for other people when I can't get my patches reviewed.) I've got no ideas how to fix this quickly, but we need to get it fixed. Patchwork only helps if we get the finished reviews cleared off - right now everything is listed as needing review, even if we've integrated it, reworked it, or rejected it. -- -Alan Coopersmith- alan.coopersm...@oracle.com Oracle Solaris Engineering - http://blogs.oracle.com/alanc ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
