improvement to patchwatcher
Hi, Is it possible to add column to the patchwatcher, whether the patch has been rejected/accepted/pending into git. Also please skip the mail entries if they do have "Re:" in the subject and do not have any patch. Mark the corresponding patch to rejected/needs more work. Thanks, VJ --------------------------------------------------------------------------------------------------------------- Fred Allen - "California is a fine place to live - if you happen to be an orange."
On Tue, Aug 26, 2008 at 1:28 PM, Vijay Kiran Kamuju <infyquest(a)gmail.com> wrote:
Is it possible to add column to the patchwatcher, whether the patch has been rejected/accepted/pending into git.
Maybe. I could probably recognize cases like http://kegel.com/wine/patchwatcher/results/358.log and say "probably already in git". But monitoring older patches and rechecking them every time there's a new git seems a bit challenging. I guess for each such patch (series), I could patch them against old git, commit, merge, and see if they're still there. Or maybe guilt can make this easier. This is beyond the scope of what I want to look at right now, but if somebody could give me a recipe, maybe.
Also please skip the mail entries if they do have "Re:" in the subject and do not have any patch. Mark the corresponding patch to rejected/needs more work.
There's two issues there: what to do about replies mistakenly sent to wine-patches (how come so many people do this?), and what to do about replies sent to wine-devel. For the latter, I plan eventually to monitor wine-devel for replies, and on the dashboard have a link to the first reply. (That won't work too well when people like me break threading...) - Dan
On Tue, Aug 26, 2008 at 3:28 PM, Vijay Kiran Kamuju <infyquest(a)gmail.com> wrote:
Hi,
Is it possible to add column to the patchwatcher, whether the patch has been rejected/accepted/pending into git.
This is extending patchwatcher too far, and it puts an unnecessary burden on Alexandre to reply to the system. In my experience, and not just with my patches, there is rarely a pending state for a patch. There is often an "I'm not sure about it" state, but that basically means the patch is rejected. The point of patchwatcher is to help the patch submitter in that he can quickly see if some of the basic requirements of a patch are not fulfilled (things he should be checking anyway).
Also please skip the mail entries if they do have "Re:" in the subject and do not have any patch. Mark the corresponding patch to rejected/needs more work.
This has already been discussed: http://winehq.org/pipermail/wine-devel/2008-August/068452.html -- James Hawkins
On Tue, Aug 26, 2008 at 4:39 PM, James Hawkins <truiken(a)gmail.com> wrote:
On Tue, Aug 26, 2008 at 3:28 PM, Vijay Kiran Kamuju <infyquest(a)gmail.com> wrote:
Hi,
Is it possible to add column to the patchwatcher, whether the patch has been rejected/accepted/pending into git.
This is extending patchwatcher too far, and it puts an unnecessary burden on Alexandre to reply to the system. In my experience, and not just with my patches, there is rarely a pending state for a patch. There is often an "I'm not sure about it" state, but that basically means the patch is rejected. The point of patchwatcher is to help the patch submitter in that he can quickly see if some of the basic requirements of a patch are not fulfilled (things he should be checking anyway).
Also please skip the mail entries if they do have "Re:" in the subject and do not have any patch. Mark the corresponding patch to rejected/needs more work.
This has already been discussed:
http://winehq.org/pipermail/wine-devel/2008-August/068452.html
-- James Hawkins
Would it be difficult though to once a day do a git fetch and attempt to re-apply all the known (good) patches to see if they've fail with patch errors (==been committed)? That seems like it might be useful.
On Tue, Aug 26, 2008 at 6:56 PM, Zachary Goldberg <zgold550(a)gmail.com> wrote:
Would it be difficult though to once a day do a git fetch and attempt to re-apply all the known (good) patches to see if they've fail with patch errors (==been committed)? That seems like it might be useful.
That's not a trivial problem. How is that useful in any way? I can imagine the case where an outside user (non-developer) wants to know the status of a submitted patch set. We already have wine-cvs archives, and source.winehq.org for that. Everyone else can just update their git tree. -- James Hawkins
On Tue, Aug 26, 2008 at 8:02 PM, James Hawkins <truiken(a)gmail.com> wrote:
On Tue, Aug 26, 2008 at 6:56 PM, Zachary Goldberg <zgold550(a)gmail.com> wrote:
Would it be difficult though to once a day do a git fetch and attempt to re-apply all the known (good) patches to see if they've fail with patch errors (==been committed)? That seems like it might be useful.
That's not a trivial problem. How is that useful in any way? I can imagine the case where an outside user (non-developer) wants to know the status of a submitted patch set. We already have wine-cvs archives, and source.winehq.org for that. Everyone else can just update their git tree.
-- James Hawkins
I may be remembering incorrectly, but If i recall wasn't there developer requests for some sort of tracking mechanism to see if their own patches had been committed? I recall some complicated discussion about using different hashes and such as inidices to try and figure out if a patch was committed.
On Tue, Aug 26, 2008 at 7:03 PM, Zachary Goldberg <zgold550(a)gmail.com> wrote:
I may be remembering incorrectly, but If i recall wasn't there developer requests for some sort of tracking mechanism to see if their own patches had been committed? I recall some complicated discussion about using different hashes and such as inidices to try and figure out if a patch was committed.
I don't know if any developer ever asked for that feature. My question still stands. How is that useful at all, especially for a developer? -- James Hawkins
On Tue, Aug 26, 2008 at 5:10 PM, James Hawkins <truiken(a)gmail.com> wrote:
question still stands. How is that useful at all, especially for a developer?
It's of limited use, IMHO. If you care about keeping patches up to date, you'll just use git or quilt or something. If you want to know when it's committed, you can just watch wine-cvs or git itself.
On Tue, Aug 26, 2008 at 8:15 PM, Dan Kegel <dank(a)kegel.com> wrote:
On Tue, Aug 26, 2008 at 5:10 PM, James Hawkins <truiken(a)gmail.com> wrote:
question still stands. How is that useful at all, especially for a developer?
It's of limited use, IMHO. If you care about keeping patches up to date, you'll just use git or quilt or something. If you want to know when it's committed, you can just watch wine-cvs or git itself.
I guess I'm remembering incorrectly. I agree on your points about using alternatives.
2008/8/27 Zachary Goldberg <zgold550(a)gmail.com>:
I may be remembering incorrectly, but If i recall wasn't there developer requests for some sort of tracking mechanism to see if their own patches had been committed? I recall some complicated discussion about using different hashes and such as inidices to try and figure out if a patch was committed.
It was probably about knowing if a patch was explicitly rejected or just pending. In practice, if it's been more than a few days the patch is almost always rejected, so I don't think it matters all that much.
"James Hawkins" <truiken(a)gmail.com> writes:
On Tue, Aug 26, 2008 at 6:56 PM, Zachary Goldberg <zgold550(a)gmail.com> wrote:
Would it be difficult though to once a day do a git fetch and attempt to re-apply all the known (good) patches to see if they've fail with patch errors (==been committed)? That seems like it might be useful.
That's not a trivial problem. How is that useful in any way? I can imagine the case where an outside user (non-developer) wants to know the status of a submitted patch set. We already have wine-cvs archives, and source.winehq.org for that. Everyone else can just update their git tree.
I think it would be quite useful. For instance it would allow sending a nice reminder to the author to rethink/resend their patch if it hasn't been applied after a week or so. It would also enable me (and other reviewers) to always get an up-to-date list of patches that need to be looked at, instead of having to keep track of it locally. Of course that would also require other ways to take patches off the list, for instance when an updated patch has been sent, or when someone else fixed the bug differently. I certainly agree it's not trivial to implement. -- Alexandre Julliard julliard(a)winehq.org
On Wed, Aug 27, 2008 at 2:08 AM, Alexandre Julliard <julliard(a)winehq.org> wrote:
I think it would be quite useful. For instance it would allow sending a nice reminder to the author to rethink/resend their patch if it hasn't been applied after a week or so. It would also enable me (and other reviewers) to always get an up-to-date list of patches that need to be looked at, instead of having to keep track of it locally.
Of course that would also require other ways to take patches off the list, for instance when an updated patch has been sent, or when someone else fixed the bug differently. I certainly agree it's not trivial to implement.
Many people have tried implementing patch trackers (e.g. Danny Berlin did a simple one,http://gcc.gnu.org/wiki/GCC_Patch_Tracking) but they don't seem to survive long, so doing a useful one must be a challenge. It would be hard to recognize later versions of the same patch, even with the '(try 2)' convention, since they often get split and munged while being beaten into submission. So Patchwatcher would have to have a way for developers to manually mark old patchsets as obsolete. Most developers probably wouldn't bother unless this was really, really easy. I can't imagine that happening unless, say, we start requiring patches to be submitted via a secure web interface that verifies that the author has signed the Wine Contributors Agreement etc. At which point it wouldn't be too hard to coax developers into clicking a box saying "obsoletes this old patchset". But that'd be a pretty big change in wine's workflow. - Dan
On Wed, Aug 27, 2008 at 11:44 AM, Dan Kegel <dank(a)kegel.com> wrote:
On Wed, Aug 27, 2008 at 2:08 AM, Alexandre Julliard <julliard(a)winehq.org> wrote:
I think it would be quite useful. For instance it would allow sending a nice reminder to the author to rethink/resend their patch if it hasn't been applied after a week or so. It would also enable me (and other reviewers) to always get an up-to-date list of patches that need to be looked at, instead of having to keep track of it locally.
Many people have tried implementing patch trackers (e.g. Danny Berlin did a simple one,http://gcc.gnu.org/wiki/GCC_Patch_Tracking) but they don't seem to survive long, so doing a useful one must be a challenge.
Indeed that does seem like a lot of effort to do properly and completely. Though to simply mark any individual patch as 'already applied to the tree' vs not applied isn't so difficult. Alexandre do you see any value in this bit of functionality? It is a very small bit of what real 'patch tracking' should be but not so hard to implement given the infrastructure Dan has already set up. If Dan doesn't wish to implement it but Alexandre and others may perceive value from it I will go for it.
On Wed, Aug 27, 2008 at 8:54 AM, Zachary Goldberg <zgold550(a)gmail.com>
to simply mark any individual patch as 'already applied to the tree' vs not applied isn't so difficult.
Patches gratefully accepted. The file to change is http://code.google.com/p/winezeug/source/browse/trunk/patchwatcher/dashboard... - Dan
On Wed, Aug 27, 2008 at 11:59 AM, Dan Kegel <dank(a)kegel.com> wrote:
On Wed, Aug 27, 2008 at 8:54 AM, Zachary Goldberg <zgold550(a)gmail.com>
to simply mark any individual patch as 'already applied to the tree' vs not applied isn't so difficult.
Patches gratefully accepted. The file to change is http://code.google.com/p/winezeug/source/browse/trunk/patchwatcher/dashboard... - Dan
Attached is a first attempt at a patch. On every cycle of the continuously running patchwatcher I call a new function which attempts to re-apply all patches it finds. Perhaps there is a smarter way to do this than re-applying all patches but i can't think of anything simple - the reason being we don't know how far back to check and see if Alexandre has committed. Anyway, we store the output of the patch command in a new file. The dashboard then checks the patch output to determine if the patch failed because 'patch' thinks the patch was already applied. I am sure there will need do be some churn on it before its acceptable, and I know this because I haven't tested it. I don't have a mailbox on my local machine subscribed to wine patches is problem one, although that could be fixed i guess. The second, harder to fix, issue is that running the wine test suite crashes my computer (nvidia driver bug I imagine). I don't have a secondary machine which I could setup mesa drivers and the patchwatcher. Thoughts, Dan?
On Wed, Aug 27, 2008 at 08:44:05AM -0700, Dan Kegel wrote:
It would be hard to recognize later versions of the same patch, even with the '(try 2)' convention, since they often get split and munged while being beaten into submission. So Patchwatcher would have to have a way for developers to manually mark old patchsets as obsolete.
Most developers probably wouldn't bother unless this was really, really easy. I can't imagine that happening unless, say, we start requiring patches to be submitted via a secure web interface that verifies that the author has signed the Wine Contributors Agreement etc. At which point it wouldn't be too hard to coax developers into clicking a box saying "obsoletes this old patchset". But that'd be a pretty big change in wine's workflow. - Dan
If it could handle working with GPG signatures, would that be a viable way to determine if the new patch is from the same author? I'm sure that most people could setup their mail clients to automatically sign their emails. With that supported patchwatcher could limit the ability to obsolete patches to the originating author for the general case. And a special case of allowing people with certain trusted GPG signatures to obsolete patches. When I say obsoleting patches, I'm thinking of the supporting commands embedded into the email: i.e. "!PatchWatcher patch mark obsolete <hash/id/etc>" -- Darragh "Nothing is foolproof to a sufficiently talented fool."
On Thursday 28 August 2008 14:32:27 Darragh Bailey wrote:
If it could handle working with GPG signatures, would that be a viable way to determine if the new patch is from the same author? I'm sure that most people could setup their mail clients to automatically sign their emails.
The problem with that is that signing emails makes the wine-patches archive almost worthless, as that has issues with multiple attachments. Given that patchwatcher only keeps patches around for a bit, I wonder if we really want to loose old patches that were never applied like that. Cheers, Kai -- Kai Blin WorldForge developer http://www.worldforge.org/ Wine developer http://wiki.winehq.org/KaiBlin Samba team member http://www.samba.org/samba/team/ -- Will code for cotton.
Alexandre Julliard wrote:
"James Hawkins" <truiken(a)gmail.com> writes:
On Tue, Aug 26, 2008 at 6:56 PM, Zachary Goldberg <zgold550(a)gmail.com> wrote:
Would it be difficult though to once a day do a git fetch and attempt to re-apply all the known (good) patches to see if they've fail with patch errors (==been committed)? That seems like it might be useful.
That's not a trivial problem. How is that useful in any way? I can imagine the case where an outside user (non-developer) wants to know the status of a submitted patch set. We already have wine-cvs archives, and source.winehq.org for that. Everyone else can just update their git tree.
I think it would be quite useful. For instance it would allow sending a nice reminder to the author to rethink/resend their patch if it hasn't been applied after a week or so. It would also enable me (and other reviewers) to always get an up-to-date list of patches that need to be looked at, instead of having to keep track of it locally.
Of course that would also require other ways to take patches off the list, for instance when an updated patch has been sent, or when someone else fixed the bug differently. I certainly agree it's not trivial to implement.
Next to checking wine-devel, we could also check wine-cvs and just do a simple compare of everything in the queue to the ones marked as committed in wine-cvs. The queue needs to be refreshed once a new git is available and the ones found in wine-cvs can be removed. -- Cheers, Paul.
Paul Vriens <paul.vriens.wine(a)gmail.com> wrote:
Next to checking wine-devel, we could also check wine-cvs and just do a simple compare of everything in the queue to the ones marked as committed in wine-cvs.
Or simply check the git log, which we have handy. A simple match on subject lines would probably get us 80% of way there.
The queue needs to be refreshed once a new git is available and the ones found in wine-cvs can be removed.
Or just marked as 'already applied'. - Dan
participants (9)
-
Alexandre Julliard -
Dan Kegel -
Darragh Bailey -
Henri Verbeet -
James Hawkins -
Kai Blin -
Paul Vriens -
Vijay Kiran Kamuju -
Zachary Goldberg