Summary: | patchworks should automatically notice a pushed patch | ||
---|---|---|---|
Product: | sourceware | Reporter: | Tom Tromey <tromey> |
Component: | Infrastructure | Assignee: | overseers mailing list <overseers> |
Status: | NEW --- | ||
Severity: | normal | CC: | carlos, mark, pedro, siddhesh |
Priority: | P2 | ||
Version: | unspecified | ||
Target Milestone: | --- | ||
See Also: | https://sourceware.org/bugzilla/show_bug.cgi?id=30997 | ||
Host: | Target: | ||
Build: | Last reconfirmed: |
Description
Tom Tromey
2023-01-24 14:29:24 UTC
(In reply to Tom Tromey from comment #0) > Consider this patch: > > https://patchwork.sourceware.org/project/gdb/patch/20230122212801.1926063-1- > tom@tromey.com/ > > I used git send-email to send this patch and patchworks dutifully > logged it. Then I pushed the patch -- but patchworks did nothing, > I had to go into the web interface to explicitly mark it as committed. > > I think patchworks ought to notice this situation. This is easier said than done unfortunately. (1) Out of the box feature. This is not functionality that patchwork provides. I would be an RFE for patchwork upstream (https://github.com/getpatchwork/patchwork/). (2) Commit cleanup script. Alternatively, in glibc we run a script to compare commits against patchwork patches and close those that have been committed. The first problem we saw here was that it requires discipline from the community to commit *exactly* what was posted and agreed upon in order to make the checking less error prone. In glibc we have adopted a stronger policy that you should only commit what the author provided, or ask the author to change it. My own opinion is that it is disrespectful to change what the author is submitting. The only allowed changes are additions of Reviewed-by, Tested-by tags in the commit message. In either (1) or (2) I think this is up to the projects to run the scripts that they need. FWIW, glibc uses a variant of this script: https://gist.github.com/siddhesh/b7be0970d8a70e5d68c36f3957ae19a4 I basically run it as a cron job. I think this could be adapted by gdb too with the caveats that Carlos mentioned in comment 1. (In reply to Carlos O'Donell from comment #1) > The first problem we saw here was that it requires discipline from the > community to commit *exactly* what was posted and agreed upon in order to > make the checking less error prone. This would work fine for [pushed] patches, which seems good enough for this ticket. For other patches, this seems like more work because it means there will have to be one final series sent, but patchworks is also bad about removing old series. > In either (1) or (2) I think this is up to the projects to run the scripts > that they need. Are you saying this shouldn't be a sourceware PR? I can close it if so. (In reply to Tom Tromey from comment #3) > (In reply to Carlos O'Donell from comment #1) > > > The first problem we saw here was that it requires discipline from the > > community to commit *exactly* what was posted and agreed upon in order to > > make the checking less error prone. > > This would work fine for [pushed] patches, which seems good enough for > this ticket. yeah, one step at a time. Siddhesh script could be run under gdbadmin or we could do it from the buildbot since we build each gdb commit on push. > For other patches, this seems like more work because it means there > will have to be one final series sent, but patchworks is also bad about > removing old series. We could have a separate script or mail filter that checks for patches that supersede a previous patch/series (check subject is the same modulo vX+1). > > In either (1) or (2) I think this is up to the projects to run the scripts > > that they need. > > Are you saying this shouldn't be a sourceware PR? > I can close it if so. We could move it to the gdb product, but lets keep it open here to document the steps we did so other projects can adopt them if they like. (In reply to Tom Tromey from comment #3) > (In reply to Carlos O'Donell from comment #1) > > > The first problem we saw here was that it requires discipline from the > > community to commit *exactly* what was posted and agreed upon in order to > > make the checking less error prone. > > This would work fine for [pushed] patches, which seems good enough for > this ticket. > > For other patches, this seems like more work because it means there > will have to be one final series sent, but patchworks is also bad about > removing old series. Yeah I've been dragging my feet on this; I've been meaning to write a script that automatically marks outdated patches as superseded (e.g. with a matching subject line) but I never got to doing it. If someone does, it would be great to use it for glibc too because right now we clean up manually. (In reply to Siddhesh Poyarekar from comment #5) > Yeah I've been dragging my feet on this; I've been meaning to write a script > that automatically marks outdated patches as superseded (e.g. with a > matching subject line) but I never got to doing it. If someone does, it > would be great to use it for glibc too because right now we clean up > manually. For gdb it would be fine to require gerrit-like Change-Id tags. We don't care about the noise (a bunch of patches already come with these anyway), and at least I personally care a lot about automation. I once tried patching patchworks for this, and I could maybe dig up my patch, but I didn't have a good way to test it. > For gdb it would be fine to require gerrit-like Change-Id tags.
+1
|