Re: Patchwork (was Re: Governance revisited)
From: Vitaliy Margolen <wine-devel(a)kievinfo.com>
So in a sense you will require some one to respond for any incoming e-mail to wine-patches. And if no one does, Alexandre don't get to see the status?
Not sure I understand what you mean. If no-one responds to the patch on wine-devel the patch would remain queued and it would show up in Alexandres list.
What if he already applied the patch? Now you slowing down what would have worked just fine before.
If the patch is already applied it would be marked as such in the database. When a message is sent to wine-devel after the fact, it won't get magically un-applied.
Ok now you making it dependant on an e-mail client. Yet that's exactly what we are trying avoid with GIT.
Actually, my preference would be to do patch management via a web-based interface. But that's just my preference, I assume a lot of other people prefer the mailinglist method. So let's try to support both.
Not really. You can't make some one to review something that don't understand. Or if they are busy/on vacation/away from PC/etc.
I'm not trying to change the review process, it would remain as-is. The only addition would be a bot which could do some checks (e.g. does the patch apply cleanly?). But that bot could be introduced anyway, even if Patchwork isn't introduced, so I guess it's not an integral part of the proposed system.
So in the end you'll end up with either huge queue that no one wants to touch. Or a "clean up" jobs that will once in a while go and mark all patches as old and will require authors to resubmit. How's that better then what we use now?
If no-one touches a patch, it would end up on Alexandres plate. He could review the patch himself or ask a subsystem maintainer to review it. Again, that's not different from the current system. If the queue of patches waiting for commit grows without bounds, that's a clear indication that Alexandre is overloaded and the project should find ways to remove some of his workload. The same would happen without a patch management system, but perhaps it wouldn't be as visible, so I'm chalking this one up as an added benefit of a patch management system. The system would also be self-cleaning. If a patch has been in RFC status too long it would be deleted from the queue (with appropriate warning email to the author) http://www.winehq.org/pipermail/wine-devel/2006-September/051114.html
When people well send out right hacks and expect some one to tell then what they really should do. Current system allows to no waste any time on such craft.
We seem to have fundamentally different views of the "peripheral" (non-core) Wine developers. You seem to view them as potentially bad guys, going out of their way to submit hacks and decrease the quality of Wine code as much as possible, people you'd rather see leave. I see people willing to donate their time to improve Wine. Sure, they make mistakes but at least they make an effort.
No, by requiring some one to respond you making them responsible (at least until they respond). Of course responses like "sucks" wouldn't be nice, so some one who does understand the subject will have to spend their time to understand the patch, write a review of the patch and send it.
I don't get your point here. Isn't the purpose of reviewing to actually review the patch?
And you want that ASAP! Which means whenever patch arrives in wine-patches some one (most likely more then one person) will stop everything they are doing, and start writing a review?
No, I don't want to change the review process. It can remain just as it is now. My objective is to improve Wine by maximizing the number of patches of acceptable quality. In my opinion, this can be done by: 1) assuring no patches get lost 2) assuring an author gets informed about why his patch is not acceptable in its current form so he can improve it. Ge van Geldorp.
Ge van Geldorp <ge <at> gse.nl> writes:
From: Vitaliy Margolen <wine-devel <at> kievinfo.com>
So in a sense you will require some one to respond for any incoming e-mail to wine-patches. And if no one does, Alexandre don't get to see the status?
Not sure I understand what you mean. If no-one responds to the patch on wine-devel the patch would remain queued and it would show up in Alexandres list.
Hi, Just to mix in this discussion my 0.02, I am one of those would-be submitters who hasn't done so because I don't want to submit crappy code. I've been viewing at loads of patches to see what they need to conform to. In my opinion, this patchwork system combined with the proposal of mentors/domain experts (as also described by someone in this thread) would mean that once I submit a patch which does not comply with the rules (which I would not know as starting contributer), I at least would get a message back from the bot which tells me why it was obvious wrong. Perhaps based on this I would receive a link to a wiki page where these rules are mentioned? If the mentors get multiple patches from different patches with a generic mistake, perhaps a new rule could be added to the bot (and the wiki) to prevent new contributers from making this mistake in the future. Worst case scenario: The bot does not find anything wrong, the mentor would review and approve, but Alexandre would not approve. In this case at least it would be rejected and he would have to make a (minor) comment as to why. But this seems to be the current situation. For me, the positive side would be that I could have an overview of my patches, learn from the feedback and improve them. I'm not saying that that is currently not possible, but since I am only able to work on this after working hours, it would take more of a bite out of my time available than if this patchwork system would be there, which increases the chances of me actually submitting patches. There is a lot of information for developers in the wiki already, but as a new person it would be nice to have some tools/pages to guide you through the process. I am very much in favour of the mentor initiative, since they could filter/improve the patches for specific areas before Alexandre needs to be bothered and provide valuable feedback to me. Okay, system or not, I will continue to try to contribute. Even if it is just by testing applications or translating strings. Frans Kool.
Ge van Geldorp wrote:
My objective is to improve Wine by maximizing the number of patches of acceptable quality. In my opinion, this can be done by:
1) assuring no patches get lost 2) assuring an author gets informed about why his patch is not acceptable in its current form so he can improve it.
That sounds good, but it's not reasonable to put the responsibility on Alexandre, as he has enough work already. From your other mail:
mention the time it costs the author. Shouldn't we be looking at the productivity of everyone involved in Wine development and not just at Alexandres productivity (although I acknowledge his special position)? > I'm a bit surprised (and, to be honest, also a little bit annoyed) about the low value you seem to place on the time contributed by the developers.
With a single maintainer system, costs to patch submitters and authors are much less crucial to a working system than costs to the single maintainer. Spreading the workload, so the many do more work, is the only way to improve the system. We agree that encouraging more reviewers is a good thing, so how about focusing on ways to get more people to review patches? Mike
Hello Mike,
From: Mike McCormack [mailto:mike(a)codeweavers.com]
That sounds good, but it's not reasonable to put the responsibility on Alexandre, as he has enough work already.
Unless you can read Alexandres mind, he's really the only one who can tell what he didn't like about a certain patch. Hopefully he'll get help from others to weed out obviously incorrect patches, but in the end it's his decision.
With a single maintainer system, costs to patch submitters and authors are much less crucial to a working system than costs to the single maintainer.
Which is why I made the remark about Alexandres special position. It doesn't mean (at least I hope it doesn't mean) that costs to authors are not important at all.
We agree that encouraging more reviewers is a good thing, so how about focusing on ways to get more people to review patches?
One doesn't preclude the other. I indeed agree it would be good to get more people to review patches, but I also think that's not a complete solution. The results of the review (either by peers, subsystem maintainers or Alexandre) needs to be communicated back to the author. Focusing solely on review doesn't solve the problem of patches getting lost either. Ge van Geldorp.
Mike McCormack wrote:
Ge van Geldorp wrote:
My objective is to improve Wine by maximizing the number of patches of acceptable quality. In my opinion, this can be done by:
1) assuring no patches get lost 2) assuring an author gets informed about why his patch is not acceptable in its current form so he can improve it.
That sounds good, but it's not reasonable to put the responsibility on Alexandre, as he has enough work already.
I have been watching this thread with keen interest. Alexandre does not HAVE to respond to that patch, he can silently ignore it just like he can now. The only difference with Patchwork would be that after a certain time with no comments and no commits, the patch would be removed from the queue and the submitter would get an email warning. regards, Jakob Eriksson
From: Jakob Eriksson [mailto:jakob(a)vmlinux.org]
I have been watching this thread with keen interest.
Alexandre does not HAVE to respond to that patch, he can silently ignore it just like he can now.
The only difference with Patchwork would be that after a certain time with no comments and no commits, the patch would be removed from the queue and the submitter would get an email warning.
Actually, that's not how I intended things to work. The automatic removal from the queue would only happen if the patch had a RFC status, i.e. if action is expected from the patch submitter. If the patch is unopposed and just waiting in the queue, it should stay there. It's reasonable to tell a submitter "you seem to have lost interest in this patch, it has been waiting on action from you for [a month, whatever] but nothing has happened, therefore it will be removed from the queue". Sending a warning message "your patch is going to be dropped without explanation" is no improvement over the current situation. Gé van Geldorp.
Ge van Geldorp wrote:
Actually, that's not how I intended things to work. The automatic removal from the queue would only happen if the patch had a RFC status, i.e. if action is expected from the patch submitter. If the patch is unopposed and just waiting in the queue, it should stay there. It's reasonable to tell a submitter "you seem to have lost interest in this patch, it has been waiting on action from you for [a month, whatever] but nothing has happened, therefore it will be removed from the queue". Sending a warning message "your patch is going to be dropped without explanation" is no improvement over the current situation.
Ok, I misunderstood. These things are tricky to comprehend and even harder to discuss. :) // Jakob
participants (4)
-
Frans Kool -
Ge van Geldorp -
Jakob Eriksson -
Mike McCormack