Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
On Monday 25 August 2008 23:58:17 Scott Ritchie wrote: > James Hawkins wrote: > > On Sun, Aug 24, 2008 at 5:06 PM, Henri Verbeet <[EMAIL PROTECTED]> wrote: > >> Something else... I sometimes reply to patches, and right now that > >> seems to cause patchwatcher to complain that my mail doesn't contain a > >> patch. Would it be possible to do something about that? I suppose > >> patchwatcher could ignore replies without patch. > > > > Is your reply sent to wine-patches? Any replies to a patch should be > > sent to wine-devel unless you're sending another patch. > > Shouldn't wine-patches have it's reply-to field set to wine-devel then? It has. Cheers, Kai -- Kai Blin WorldForge developer http://www.worldforge.org/ Wine developerhttp://wiki.winehq.org/KaiBlin Samba team member http://www.samba.org/samba/team/ -- Will code for cotton. signature.asc Description: This is a digitally signed message part.
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
Dan Kegel wrote: > On Mon, Aug 25, 2008 at 2:05 PM, Kai Blin <[EMAIL PROTECTED]> wrote: > #patchwatcher approve Such a directive would only be recognized if it's alone on a line. >>> The usual positive reply is 'ACK'. I believe it would be simplest to >>> grep the reply message for ACK to see if a reply is positive. >>> >> Dunno, I find myself replying to patches that fix bugs I worked on myself or >> that were in my code with things like "good catch". I agree with Alexandre >> that cases like that are seldom enough that we can probably ignore them. >> > > Around where I work, LGTM (looks good to me) is the reply that > denotes approval. And then there's the Apache convention of +1. > Anyway, I'll recognize one or more of those if and when I get around > to adding a wine-devel listener. Might be a couple weeks. > > Dan: Thank you for adding this. James McKenzie
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
On Mon, Aug 25, 2008 at 4:58 PM, Scott Ritchie <[EMAIL PROTECTED]> wrote: > James Hawkins wrote: >> On Sun, Aug 24, 2008 at 5:06 PM, Henri Verbeet <[EMAIL PROTECTED]> wrote: >>> Something else... I sometimes reply to patches, and right now that >>> seems to cause patchwatcher to complain that my mail doesn't contain a >>> patch. Would it be possible to do something about that? I suppose >>> patchwatcher could ignore replies without patch. >>> >> >> Is your reply sent to wine-patches? Any replies to a patch should be >> sent to wine-devel unless you're sending another patch. >> > > Shouldn't wine-patches have it's reply-to field set to wine-devel then? > It is, but for some reason wine-patches is CC'ed instead of the patch sender when you reply-all. -- James Hawkins
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
James Hawkins wrote: > On Sun, Aug 24, 2008 at 5:06 PM, Henri Verbeet <[EMAIL PROTECTED]> wrote: >> Something else... I sometimes reply to patches, and right now that >> seems to cause patchwatcher to complain that my mail doesn't contain a >> patch. Would it be possible to do something about that? I suppose >> patchwatcher could ignore replies without patch. >> > > Is your reply sent to wine-patches? Any replies to a patch should be > sent to wine-devel unless you're sending another patch. > Shouldn't wine-patches have it's reply-to field set to wine-devel then? Thanks, Scott Ritchie
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
On Monday 25 August 2008 23:20:40 Dan Kegel wrote: > Around where I work, LGTM (looks good to me) is the reply that > denotes approval. And then there's the Apache convention of +1. > Anyway, I'll recognize one or more of those if and when I get around > to adding a wine-devel listener. Might be a couple weeks. Hm, it's getting harder and harder to keep up with this in buildbot. :) Cheers, Kai -- Kai Blin WorldForge developer http://www.worldforge.org/ Wine developerhttp://wiki.winehq.org/KaiBlin Samba team member http://www.samba.org/samba/team/ -- Will code for cotton. signature.asc Description: This is a digitally signed message part.
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
On Mon, Aug 25, 2008 at 2:05 PM, Kai Blin <[EMAIL PROTECTED]> wrote: >> > #patchwatcher approve >> > >> > Such a directive would only be recognized if it's alone on a line. >> >> The usual positive reply is 'ACK'. I believe it would be simplest to >> grep the reply message for ACK to see if a reply is positive. > > Dunno, I find myself replying to patches that fix bugs I worked on myself or > that were in my code with things like "good catch". I agree with Alexandre > that cases like that are seldom enough that we can probably ignore them. Around where I work, LGTM (looks good to me) is the reply that denotes approval. And then there's the Apache convention of +1. Anyway, I'll recognize one or more of those if and when I get around to adding a wine-devel listener. Might be a couple weeks. - Dan
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
On Monday 25 August 2008 20:46:51 James Hawkins wrote: > On Mon, Aug 25, 2008 at 1:11 PM, Ken Thomases <[EMAIL PROTECTED]> wrote: > > > > #patchwatcher approve > > > > Such a directive would only be recognized if it's alone on a line. > > The usual positive reply is 'ACK'. I believe it would be simplest to > grep the reply message for ACK to see if a reply is positive. Dunno, I find myself replying to patches that fix bugs I worked on myself or that were in my code with things like "good catch". I agree with Alexandre that cases like that are seldom enough that we can probably ignore them. Cheers, Kai -- Kai Blin WorldForge developer http://www.worldforge.org/ Wine developerhttp://wiki.winehq.org/KaiBlin Samba team member http://www.samba.org/samba/team/ -- Will code for cotton. signature.asc Description: This is a digitally signed message part.
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
On Mon, Aug 25, 2008 at 2:46 PM, James Hawkins <[EMAIL PROTECTED]> wrote: > On Mon, Aug 25, 2008 at 1:11 PM, Ken Thomases <[EMAIL PROTECTED]> wrote: >> On Aug 25, 2008, at 1:08 PM, Alexandre Julliard wrote: >> >>> "Dan Kegel" <[EMAIL PROTECTED]> writes: >>> On Mon, Aug 25, 2008 at 8:47 AM, Alexandre Julliard <[EMAIL PROTECTED]> wrote: > patchwatcher should watch for replies on wine-devel, and > link the reply to the patch Yes. The other similar patchwatching systems I found do this, and ours should, too. > and mark the patch as needing further action on the submitter's > part. Even if the message says "Great job!"? >>> >>> Sure, that's not the usual case. In most cases when a patch gets a >>> reply >>> it's because it will need changes. There could of course be a way >>> for a >>> submitter to mark the patch as still valid, but the first action >>> should >>> be to take the patch off the list of committable patches. >> >> Perhaps replies meant to approve of a patch could include a special >> textual directive to indicate that patchwatcher shouldn't block the >> patch. Something like: >> >> #patchwatcher approve >> >> Such a directive would only be recognized if it's alone on a line. >> > > The usual positive reply is 'ACK'. I believe it would be simplest to > grep the reply message for ACK to see if a reply is positive. > > -- > James Hawkins > It might be useful if the patch emails themselves contained the patchwatcher documentation. If the mail server could append some instructions onto the patch email we could add some simple directions on how to reply to the patch with an ack etc. Chris
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
On Mon, Aug 25, 2008 at 1:11 PM, Ken Thomases <[EMAIL PROTECTED]> wrote: > On Aug 25, 2008, at 1:08 PM, Alexandre Julliard wrote: > >> "Dan Kegel" <[EMAIL PROTECTED]> writes: >> >>> On Mon, Aug 25, 2008 at 8:47 AM, Alexandre Julliard >>> <[EMAIL PROTECTED]> wrote: patchwatcher should watch for replies on wine-devel, and link the reply to the patch >>> >>> Yes. The other similar patchwatching systems I found do this, >>> and ours should, too. >>> and mark the patch as needing further action on the submitter's part. >>> >>> Even if the message says "Great job!"? >> >> Sure, that's not the usual case. In most cases when a patch gets a >> reply >> it's because it will need changes. There could of course be a way >> for a >> submitter to mark the patch as still valid, but the first action >> should >> be to take the patch off the list of committable patches. > > Perhaps replies meant to approve of a patch could include a special > textual directive to indicate that patchwatcher shouldn't block the > patch. Something like: > > #patchwatcher approve > > Such a directive would only be recognized if it's alone on a line. > The usual positive reply is 'ACK'. I believe it would be simplest to grep the reply message for ACK to see if a reply is positive. -- James Hawkins
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
On Aug 25, 2008, at 1:08 PM, Alexandre Julliard wrote: > "Dan Kegel" <[EMAIL PROTECTED]> writes: > >> On Mon, Aug 25, 2008 at 8:47 AM, Alexandre Julliard >> <[EMAIL PROTECTED]> wrote: >>> patchwatcher should watch for replies on wine-devel, and >>> link the reply to the patch >> >> Yes. The other similar patchwatching systems I found do this, >> and ours should, too. >> >>> and mark the patch as needing further action on the submitter's >>> part. >> >> Even if the message says "Great job!"? > > Sure, that's not the usual case. In most cases when a patch gets a > reply > it's because it will need changes. There could of course be a way > for a > submitter to mark the patch as still valid, but the first action > should > be to take the patch off the list of committable patches. Perhaps replies meant to approve of a patch could include a special textual directive to indicate that patchwatcher shouldn't block the patch. Something like: #patchwatcher approve Such a directive would only be recognized if it's alone on a line. -Ken
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
"Dan Kegel" <[EMAIL PROTECTED]> writes: > On Mon, Aug 25, 2008 at 8:47 AM, Alexandre Julliard <[EMAIL PROTECTED]> wrote: >> patchwatcher should watch for replies on wine-devel, and >> link the reply to the patch > > Yes. The other similar patchwatching systems I found do this, > and ours should, too. > >> and mark the patch as needing further action on the submitter's part. > > Even if the message says "Great job!"? Sure, that's not the usual case. In most cases when a patch gets a reply it's because it will need changes. There could of course be a way for a submitter to mark the patch as still valid, but the first action should be to take the patch off the list of committable patches. -- Alexandre Julliard [EMAIL PROTECTED]
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
On Mon, Aug 25, 2008 at 8:47 AM, Alexandre Julliard <[EMAIL PROTECTED]> wrote: > patchwatcher should watch for replies on wine-devel, and > link the reply to the patch Yes. The other similar patchwatching systems I found do this, and ours should, too. > and mark the patch as needing further action on the submitter's part. Even if the message says "Great job!"? - Dan
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
Alexandre Julliard wrote: > Kai Blin <[EMAIL PROTECTED]> writes: > >> On Monday 25 August 2008 00:06:29 Henri Verbeet wrote: >>> Something else... I sometimes reply to patches, and right now that >>> seems to cause patchwatcher to complain that my mail doesn't contain a >>> patch. Would it be possible to do something about that? >> I've got to agree with James on that one. Replies to patches should contain >> a >> patch or be to wine-devel. Patchwatcher helps to remind people on a policy >> here. This will also help people who forgot to attach a patch to their >> email, >> so I think it's a good idea to not ignore emails without patches. > > Even better patchwatcher should watch for replies on wine-devel, and > link the reply to the patch and mark the patch as needing further action > on the submitter's part. > Nice way of blocking a patch ;-). -- Cheers, Paul.
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
Kai Blin <[EMAIL PROTECTED]> writes: > On Monday 25 August 2008 00:06:29 Henri Verbeet wrote: >> Something else... I sometimes reply to patches, and right now that >> seems to cause patchwatcher to complain that my mail doesn't contain a >> patch. Would it be possible to do something about that? > > I've got to agree with James on that one. Replies to patches should contain a > patch or be to wine-devel. Patchwatcher helps to remind people on a policy > here. This will also help people who forgot to attach a patch to their email, > so I think it's a good idea to not ignore emails without patches. Even better patchwatcher should watch for replies on wine-devel, and link the reply to the patch and mark the patch as needing further action on the submitter's part. -- Alexandre Julliard [EMAIL PROTECTED]
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
On Monday 25 August 2008 00:06:29 Henri Verbeet wrote: > Something else... I sometimes reply to patches, and right now that > seems to cause patchwatcher to complain that my mail doesn't contain a > patch. Would it be possible to do something about that? I've got to agree with James on that one. Replies to patches should contain a patch or be to wine-devel. Patchwatcher helps to remind people on a policy here. This will also help people who forgot to attach a patch to their email, so I think it's a good idea to not ignore emails without patches. Cheers, Kai -- Kai Blin WorldForge developer http://www.worldforge.org/ Wine developerhttp://wiki.winehq.org/KaiBlin Samba team member http://www.samba.org/samba/team/ -- Will code for cotton. signature.asc Description: This is a digitally signed message part.
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
On Sun, Aug 24, 2008 at 5:06 PM, Henri Verbeet <[EMAIL PROTECTED]> wrote: > Something else... I sometimes reply to patches, and right now that > seems to cause patchwatcher to complain that my mail doesn't contain a > patch. Would it be possible to do something about that? I suppose > patchwatcher could ignore replies without patch. > Is your reply sent to wine-patches? Any replies to a patch should be sent to wine-devel unless you're sending another patch. -- James Hawkins
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
Something else... I sometimes reply to patches, and right now that seems to cause patchwatcher to complain that my mail doesn't contain a patch. Would it be possible to do something about that? I suppose patchwatcher could ignore replies without patch.
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
2008/8/24 Dan Kegel <[EMAIL PROTECTED]>: > On Sun, Aug 24, 2008 at 9:38 AM, Dan Kegel <[EMAIL PROTECTED]> wrote: >> The script now forwards patches that pass all tests to the mailing list >> http://groups.google.com/group/wine-patches-filtered >> so potentially Alexandre could start looking only at that >> list > > Hmm. At the moment, patchwatcher will forward parts > of a patch series even if some of the series fails; > it should probably only forward a patch series if there > are no failures in any of its members. > Depends, often if a patch in a series fails the earlier patches can still be applied. Also, that a patch is part of a series doesn't necessarily mean it depends on the previous patches, sending patches as a series is just the easiest thing to do when you've got multiple patches.
Re: Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
On Sun, Aug 24, 2008 at 9:38 AM, Dan Kegel <[EMAIL PROTECTED]> wrote: > The script now forwards patches that pass all tests to the mailing list > http://groups.google.com/group/wine-patches-filtered > so potentially Alexandre could start looking only at that > list Hmm. At the moment, patchwatcher will forward parts of a patch series even if some of the series fails; it should probably only forward a patch series if there are no failures in any of its members.
Patchwatcher status: regenerates configure, notifies authors on failure, filtered patches mailing list
Patchwatcher now regenerates configure and makefiles after each patch, so it can handle patches that change configure.ac. I have enabled email notification to authors of patches that don't build. The script now forwards patches that pass all tests to the mailing list http://groups.google.com/group/wine-patches-filtered so potentially Alexandre could start looking only at that list (though it probably only has two nines of reliability at this point, as patchwatcher doesn't handle every case yet, e.g. it doesn't handle partial resends of patch series).