Re: Pain points in Git's patch flow
Junio C Hamano writes: > So here is a real-life example. > > Let's say somebody is looking at a "gentle ping" [*1*] > > znh> The patch seems to have fallen into the crack. > zhn> Jeff and Junio, willing to help? > > How would we figure out what happened to the patch today without > visiting patchwork would be: > ... > Now, how can patchwork improve the above reviewer experience, out > of the box and possibly with new helpe rools around it? Also, it would be ideal if it is made easy for willing reviewers with excess bandwidth to preemptively find and review patches that need reviewing. I think your original write-up upthread covered this use case sufficiently. Thanks. ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Pain points in Git's patch flow
Jonathan Nieder writes: > 3. Do you think patchwork goes in a direction that is likely to help > with these? So here is a real-life example. Let's say somebody is looking at a "gentle ping" [*1*] znh> The patch seems to have fallen into the crack. zhn> Jeff and Junio, willing to help? How would we figure out what happened to the patch today without visiting patchwork would be: 1. Visit the message at lore.kernel.org/git/ [*1*] 2. Notice that it is a response to a message, and click the link to be taken to [*2*] 3. Notice that nobody commented on the patch. 4. Type "f:zhening ref-filter" to the search box and search, with suspicion that this was an updated version of something. 5. Click one of them in the result [*3*] 6. This time, we can tell that this seemed to have had two earlier iterations, and after reading the discussion through, the last one changed the course in a major way. Not just a new helper introduced in the earlier rounds has gone away, but an existing helper got removed. 7. All comments in the discussion for the earlier two rounds can be read as supporting the new direction the latest round takes. 8. The fact remains that even if the direction has been endorsed (see 7. above) nobody took a look at the implementation for the latest round. 9. Make the final verdict. I use my newsreader to do pretty much the equivalent of the above without hitting https://lore.kernel.org/git/ but the above is written to use the web interface, in order to make it reproducible more easily by anybody on the list. Now, how can patchwork improve the above reviewer experience, out of the box and possibly with new helpe rools around it? I can see #3 would immediately become obvious, and I hope #4-#5 would become unnecessary. Anything else? At steps #6 and #7, there is human judgment involved that may not be automatable, but would there be some mechanism to make it easy to help these steps if the user visits patchwork (instead of staying in my newsreader or web interface to the lore archive)? I am of course not expecting to automate step #9 ;-) It would be nice though. Thanks. [References] *1* https://lore.kernel.org/git/caoltt8tis5yjg8ur0c-i0bnqifqvlxvdgxuqj-wcp6jjqpu...@mail.gmail.com/ *2* https://lore.kernel.org/git/pull.928.git.1617975348494.gitgitgad...@gmail.com/ *3* https://lore.kernel.org/git/pull.927.v2.git.1617809209164.gitgitgad...@gmail.com/ ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Pain points in Git's patch flow
Jonathan Nieder writes: > That reminded me that it would be useful preparation to collect > descriptions of pain points we are having with our existing patch > flow. As the maintainer, I want to ensure that what I queue on 'seen' is kept reasonably up to date. Not picking up the latest every time a topic is rerolled is OK, but before declaring the topic will hit 'next' in a few days, it must be (1) the latest and (2) the greatest (meaning: what reviewers are happy with). This requires a few features from any tracking system. It must be able to: - tell which round of patches are in 'seen'. - tell which e-mail messages are the newer round than what is queued, and which ones are the latest round. - tell which patch have been commented on, and been updated in response. - tell which patch have been positively accepted with an Ack or a Reviewed-by, and if a patch in a newer round is identical to an already accepted one (in which case, reviewers would not bother to send "this step still looks good to me"). The system I use for the first two points is to rely on the list archive and the mapping from each individual commit made out of a patch on the list (implemented as git notes, that records a blob with the Message-Id for each commit [*1*]). So $ git log --notes=amlog --no-merges --oneline master..$topic would give a list of commits with the original Message-ID, and I can see which piece of e-mail each commit came from. The third and fourth are maintained mostly manual, with me keeping notes in the draft of "What's cooking" report (which I send out from time to time). Having to notice, pick up and squeeze in Acked-by's and Reviewed-by's is quite painful and cumbersome, especially for a long series, and the buggy "git rebase -x" does not help, either [*2*]. If we run a patchwork instance for our project, the first two could be largely automated. Automation built around Patchwork should be able to, or at least should be able to help me to: - notice when a new round of an existing topic is posted. - fetch the "amlog" notes, together with a copy of daily 'seen', to see if a topic that is queued has an update, and notify me and others when the topic queued is stale [*3*]. - tie a step in the latest round with a corresponding step in the previous round, and show Ack's and Reviewed-By's that are still valid [*4*]. [Footnotes] *1* "git fetch https://github.com/gitster/git +refs/notes/amlog:refs/notes/amlog" should give you a copy of this database. Then, for example you can ask where a commit came from: $ git show -s --notes=amlog format="%N%s" 61a7660516 Message-Id: reftable: document an alternate cleanup method on Windows Note that this is not a one-to-one mapping. I may initially apply patches to an inappropriate base and push the integration result out that has it in 'seen', but I may realize that the series needs to be queued on a different commit and rebase the topic the next day. Both commits before and after such a rebasing have come from the single piece of e-mail, so you can say "this commit came from this message", but it is impossible to expect a single answer to "which commit is the result of this message"---there will be multiple. Strictly speaking, when two rounds of the same topic had patches that were unchanged between the iterations in their earliest parts, two pieces of e-mail may convey the same patch, so in the ideal world, it might be more useful to record "this commit came from this and that messges, both of which record an identical patch". I currently do not do so, though. *2* It would be ideal if "rebase -i -x 'add-trailer -r peff@'" can be used to stop at each commit, run the 'add-trailer -r peff@' script that amends HEAD to add "Reviewed-by: peff@", and continue, while honoring the "notes.rewriteref" configuration variable (in my repository, set to "refs/notes/amlog"). That way, I can queue with "git am", at which time "amlog" gets populated to map each commit to the original message, find Reviewed-by: and do the above rebase, while carrying the message IDs to resulting commits. Alas, "rebase -i -x" is buggy and loses the notes during this process (doing s/pick/reword/ and manually squeezing Reviewed-by: into the log message is a poor but workable workaround). cf. https://lore.kernel.org/git/xmqq8s6tcuxc.fsf@gitster.g/ *3* It is perfectly normal if a topic is left stale, if the newer iteration breaks integration. So the stale notification going directly to a contributor from an automated system would not help very much, but it needs to come with the reason why it is kept out of 'seen', which must be supplied by human, not by an automation. If an automation around Patchwork can send, instead of "hey, here is an updated series available" notification, a mbox
Re: Pain points in Git's patch flow
Junio C Hamano writes: > So, I dunno. We seem to suffer from the same lack of good starter > tasks before each GSoC begins. And long after sending the response, I realize that this has very little to do with "Git's patch flow" issue that you wanted to discuss. Helping newbies wet their toes may be a topic worth discussing, but that is not the focus of this thread. Let me send a response to describe my pain points separately. Sorry for the noise. ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Pain points in Git's patch flow
Bagas Sanjaya writes: > There is no lists of "beginner-friendly" issues that can be worked on by > new contributors. They had to search this ML archive for bug report > issues and determine themselves which are beginner-friendly. Yeah, looking for "#leftoverbits" or "low-hanging" on the list archive is often cited as a way, and it does seem easy enough to do. You go to https://lore.kernel.org/git/, type "leftoverbits" or "low-hanging" in the text input and press SEARCH. But that is only half of the story. Anybody can throw random ideas and label them "#leftoverbits" or "low-hanging fruit", but some of these ideas might turn out to be ill-conceived or outright nonsense. Limiting search to the utterances by those with known good taste does help, but as a newbie, you do not know who these people with good taste are. It might help to have a curated list of starter tasks, but I suspect that they tend to get depleted rather quickly---by definition the ones on the list are easy to do and there is nothing to stop an eager newbie from eating all of them in one sitting X-(. So, I dunno. We seem to suffer from the same lack of good starter tasks before each GSoC begins. ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] parser: Unmangle From: headers that have been mangled for DMARC purposes
Jeff King writes: > This might provide an alternate solution (or vice versa). I kind of like > this one better in that it doesn't require the sender to do anything > differently (but it may be less robust, as it assumes the receiver > reliably de-mangling). I share the assessment. I also feel that relying on Reply-To: would make the result a lot less reliable (I do not have much problem with the use of X-Original-Sender, though). ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
