{"thread":{"id":"57095","subject":"Review process improvements","startedAt":"2021-12-16T22:46:31Z","lastAt":"2022-01-10T17:13:40Z","messageCount":22,"participants":["Emily Shaffer","rsbecker@nexbridge.com","Junio C Hamano","Konstantin Ryabitsev","Christian Couder","Ævar Arnfjörð Bjarmason","Mark Brown","Eric Sunshine","João Victor Bonfim","brian m. carlson","Fabian Stelzer","Stefan Hajnoczi","Matthias Aßhauer","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"444273","messageId":"YbvBvch8JcHED+A9@google.com","threadId":"57095","inReplyTo":null,"subject":"Review process improvements","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2021-12-16T22:46:21Z","receivedAt":"2021-12-16T22:46:31Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Hi folks,\n\nRecently in our team at Google I revived a conversation we've had many\ntimes before upstream - in fact, at every contributor summit that I've\nattended since I joined the project mid-2019. At each summit, we have\nsome conversation around review efficacy, process, tooling, and so on.\nIn late 2019[1], we discussed difficulty with performing reviews and\nkeeping track of them on the mailing list. In early 2020[2] we discussed\nthe unfortunate case of reviews with no responses whatsoever, and what\nmight be preventing Git contributors from reviewing more topics, more\nregularly. In late 2020[3], lots of different topics in the\ninclusion-focused contributor summit centered around reviews - tone,\ntooling, workflows, and so on.\n\nSome of those discussions resulted in changes - for example,\nGitGitGadget was improved and added to git/git, and we enjoy easy,\nnon-noisy CI runs via GitHub Actions. But some things we thought were a\ngood idea never went into practice. In the next few months, the Google\nGit team is planning to implement some of the following changes, and\nwe'd appreciate your thoughts ahead of time as well as your review later\non:\n\n1. Draft a MAINTAINERS file\nOne recurring theme from the conversations I had with the Google Git\nteam was that selecting a topic to review in the first place can\nactually be pretty time-consuming. Choosing from the series subject line\nor the \"What's cooking\" mail isn't always straightforward - it can be\nhard to tell whether the series is working on a goal you are interested\nin or in a part of the codebase you care about. It's not feasible for\nsomeone wanting to get involved to review every single series that comes\nacross the list - but it might be feasible for someone to review every\nseries in a particular subsystem or topic.\n\nIn the first half of January, I'd like to have a review out to the list\nadding a kernel-style MAINTAINERS file with a few areas of interest. To\nthat file, I'd like to add \"R:\" (\"CC me!\") lines to subsystem mailing\nlists and interested individuals. My hope is that that will make it easy\nfor someone to either add themselves as an \"R:\" or to subscribe to the\nsubsystem list, and then be able to filter their mail based on \"stuff\nI'm CC'd to\" or \"stuff sent to git-partial-clone@linux.dev\" - and then\nbe able to review every patch in that list. I'd like to create lists on\ntopics where it makes sense, too.\n\nSince the Git codebase isn't modularized into subsystems as plainly as\nthe kernel is, I don't think that the MAINTAINERS list needs to be\nmachine-parseable yet, although it would be a nice goal. Maybe over time\nwe will decide we'd rather organize the codebase differently and\nimplement a machine-parseable MAINTAINERS list to incorporate into a\nsendemail-validate hook, or something. So I envision lines looking\nsomething like this (just examples, sorry for directly calling people\nout ;) ):\n\n  Submodules\n  submodule.[ch], git-submodule.sh, builtin/submodule.c,\n    Documentation/git-submodule.txt, anything else adding a feature involving\n    submodules\n  R: git-submodules@example.com\n  R: emilyshaffer@google.com\n\n  Config improvements\n  Documentation/config/*, config.[ch], builtin/config.c, changes which add new\n    config options\n  R: emilyshaffer@google.com\n  R: avarab@gmail.com\n\nIt would be extremely useful to hear other suggestions from the mailing\nlist about subsystems which deserve a MAINTAINERS line and possibly a\nsubsystem mailing list.\n\n\n2. Draft a ReviewingPatches doc\nAnother theme we discussed was the general ambiguity around the act of\nperforming code review. How detailed should the review be? Who should be\nexamining interoperability with the rest of the codebase? Is it OK to\nreply with only \"I don't know what you're trying to do, rewrite your\ncommit message please\"?\n\nSometime in January I'd like to have a review out with an outline with\nagreement on the content and organization for a ReviewingPatches doc.\nI'm hoping that it can give some structure to review by including:\n  - How to give different types of review (maintainer vs. individual vs.\n    drive-by nits)\n  - Review best practices (Sage Sharp's article on stages of review[4],\n    CCing experts/lists from MAINTAINERS file above, short-circuiting by\n    sending comments early instead of reviewing a patch in-depth when\n    the idea seems so-so or unclear)\n  - Checklists for new contributors to use or to help make in-depth\n    review more thorough\n\nSince this is intended to be checked into Git's codebase, I expect that\nwe'll want to discuss a lot and make sure we all buy into the contents\nof that doc.\n\n3. Google Git team will review cover letters and commit messages\n   internally before sending series to the list\nI often find that when I send a patch to the list, reviewers reply\nwithout understanding the point of what I was trying to achieve in the\npatch. It sounded like this happens to some other Google Gitters as\nwell. Luckily, that's fixable by the patch author.\n\nTo try and make sure we're sending patches that are easy to understand\nand decide whether to review, we're adding a step to our process ASAP to\nrequire commit messages, cover letters, and \"---\" notes to be reviewed\nand approved by at least one other Googler before a topic goes to the\nmailing list. Those reviews don't need to be formal, but do need to\nhappen for every reroll of a series. We also will intentionally *not*\nreview the diff of changes in this private setting - reviews for code\ncorrectness, etc. should continue to happen upstream, in public.\n\nI'm mentioning that takeaway in this email to say: if you find you don't\nunderstand the purpose of a patch from a Googler, please let us know! It\nwould be really valuable for us to receive a review saying \"I think you\nmight be saying X but it's still confusing because you wrote Y\"\nso that we can incorporate the source of your confusion as we continue\nto review each other's \"accompanying context\".\n\n4. Documentation changes to encourage commit message quality\nCommit messages are important, but one comment I hear a lot from new\ncontributors to the Git project is: \"why do these people care about the\ncommit message so much? The commit message doesn't compile!\" That tells\nme that instead of explaining the importance of quality commit messages\nover and over at review time, we should be doing a better job of\nexplaining why we have high standards and helping them meet those\nstandards in documentation a contributor is likely to refer to.  On the\none hand, we want the list to be friendly and inclusive, but on the\nother hand, it’s pretty demanding for a reviewer to review patches if\nthe commit message isn’t clear enough.\n\nSubmittingPatches has a short blurb about commit message quality under\nthe \"Make separate commits for logically separate changes\" heading and a\nlonger one under \"Describe your changes well\". But I think it doesn't do\na great job explaining *why*, only explaining how. I'd like to send a\nreview sometime in January for a modification of SubmittingPatches to\ninclude a paragraph about how we use commit messages - for example, to\nidentify the motivation behind a certain line of code during 'git\nblame', or to decide whether a patch is worth reviewing. My hope is that\nthis documentation change would help convince authors that it's worth\ntheir time to write a good commit message.\n\nI'd also like to add a short hands-on section to MyFirstContribution,\nlooking up both bad and good example commits to underline the importance\nof the commit message and the way that it's used after the patch is\nmerged. I don't think it needs to be long: \"git show abcdef; can you\ntell what the commit is doing? Now git show 123456; can you tell what\nthis one is doing?\" where 'abcdef' is an example of a not-so-great\ncommit message and 123456 is an example of a fantastic commit message.\nBut I'd like to make sure that whatever commits we use as an example are\nvolunteered by the patch author, rather than chosen to be some example\nof an antipattern. So if anybody wants to volunteer an example of a\nparticularly unclear commit that they wrote, that would be incredibly\nuseful.\n\nFor both these documents, the most useful help I could see from the\nupstream mailing list would be to review and ensure we're conveying the\ngoals and the purpose of good commit message writing accurately.\n\n---\n\nFor all 4 of the above, either I or another Google Git team member will\nbe responsible for taking action. If someone else wants to volunteer or\nbeats us to the punch with a patch on list, that's certainly fine with\nme - but I'm not trying to ask for these changes from the community at\nlarge. What I am really looking for is to be held accountable: if the\nend of February 2022 rolls around and we still don't have any of the\nthings I described, feel free to raise a fuss. And I hope also to hear\nfrom you all in the form of reviews and suggestions, or shouts of \"no!\ndon't do that!\" if something sounds incorrect or counterproductive to\nyou.\n\nAs it's reaching the end of the year and the holiday season, I'm\nplanning to spend the rest of the year with family and away from my work\nemail as much as possible. Some other Googler Gitters will be working on\nand off and may chime in. But I am expecting to be able to engage more\nwith this mail in the first week of January.\n\nThanks very much for reading and discussing.\n - Emily\n\n[1] https://lore.kernel.org/git/nycvar.QRO.7.76.6.1909261253400.15067@tvgsbejvaqbjf.bet/\n(see discussion #10)\n\n[2] https://lore.kernel.org/git/6DAC1E49-9CA0-4074-867E-F22CD26C9FEB@jramsay.com.au/\n\n[3] https://docs.google.com/document/d/1g_QapVhHejHorpUzzjpT5DlPi6ovNlEK4423uYWypbU/edit#heading=h.9tcri25458a\n(See Day 2 notes; it's possible these notes are closed to those who didn't\nattend the summit)\n\n[4] https://sage.thesharps.us/2014/09/01/the-gentle-art-of-patch-review/\n"},{"id":"444275","messageId":"00b901d7f2d3$c0d2ca20$42785e60$@nexbridge.com","threadId":"57095","inReplyTo":"YbvBvch8JcHED+A9@google.com","subject":"RE: Review process improvements","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2021-12-16T23:22:06Z","receivedAt":"2021-12-16T23:22:19Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On December 16, 2021 5:46 PM, Emily Shaffer wrote:\n> 1. Draft a MAINTAINERS file\n> Since the Git codebase isn't modularized into subsystems as plainly as the\n> kernel is, I don't think that the MAINTAINERS list needs to be machine-\n> parseable yet, although it would be a nice goal.\n\nI am not sure about separate mailing lists. That gets really cumbersome as things get large. Perhaps some prefix in email based on the list.\n\nI think starting with a YAML-style MAINTAINERS file might be better than the artificial tag structure proposed even as a starting point.\n\nSomewhat like:\n\nSubmodules:\n Prefix: SUBM\n Files:\n     submodule.[ch], git-submodule.sh, builtin/submodule.c,\n     Documentation/git-submodule.txt\n Reviewers:\n   git-submodules@example.com\n   emilyshaffer@google.com\n\netc.\n\n> It would be extremely useful to hear other suggestions from the mailing list\n> about subsystems which deserve a MAINTAINERS line and possibly a\n> subsystem mailing list.\n\nSome suggestions:\n\nPlatform:\n Linux:\n  Prefix: LINUX\n  Reviewers: etc.\n NonStop:\n  Prefix: NS\n  Reviewers:\n   rsbecker@nexbridge.com (a.k.a. me!)\n etc.\n\nCompat-Layer\n Prefix: COMPAT\nPerl\nPython\nP4\nSVN\nGitk\nSigning\nFuture-Plans\nEtc.\n\n> 2. Draft a ReviewingPatches doc\n> Another theme we discussed was the general ambiguity around the act of\n> performing code review. How detailed should the review be? Who should be\n> examining interoperability with the rest of the codebase? Is it OK to reply\n> with only \"I don't know what you're trying to do, rewrite your commit\n> message please\"?\n\nPeople learn what they see, so it is important to keep review style consistent both for the actual review and to keep the review culture consistent. In some places, I have seen review \"Body of Knowledge\" documents as the review culture evolves outlining what should be in a review beside the code style guidelines.\n\n> 3. Google Git team will review cover letters and commit messages\n>    internally before sending series to the list I often find that when I send a\n> patch to the list, reviewers reply without understanding the point of what I\n> was trying to achieve in the patch. It sounded like this happens to some\n> other Google Gitters as well. Luckily, that's fixable by the patch author.\n> \n> To try and make sure we're sending patches that are easy to understand and\n> decide whether to review, we're adding a step to our process ASAP to\n> require commit messages, cover letters, and \"---\" notes to be reviewed and\n> approved by at least one other Googler before a topic goes to the mailing list.\n> Those reviews don't need to be formal, but do need to happen for every\n> reroll of a series. We also will intentionally *not* review the diff of changes in\n> this private setting - reviews for code correctness, etc. should continue to\n> happen upstream, in public.\n><snip>\n> 4. Documentation changes to encourage commit message quality Commit\n> messages are important, but one comment I hear a lot from new\n> contributors to the Git project is: \"why do these people care about the\n> commit message so much? The commit message doesn't compile!\"\n\nThis is leading down a major git enhancement RFE, which may have general applicability. One great power of git is the idempotency of the commit across files. I realize that the current style for git itself is separate commits, but I think that partly is a limitation of the commit capability itself.\n\nSuppose the cover letter is essentially a headline, with the subject being the headline headline of the commit. The commit comment could (the RFE) vary depending on the file being committed - an attribute of the node in the Merkel Tree perhaps. So the main commit node has the headline, the cover letter contents, while the file node has the main headline and the file-specific comment. In git log, you would see the headline, then the cover letter, then the file comment (if specifying a path). Just a thought. It might significantly reduce the size of history.\n\nJust some ramblings,\n-Randall\n\n"},{"id":"444287","messageId":"xmqqbl1g6qq2.fsf@gitster.g","threadId":"57095","inReplyTo":"YbvBvch8JcHED+A9@google.com","subject":"Re: Review process improvements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-17T00:05:09Z","receivedAt":"2021-12-17T00:05:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer <emilyshaffer@google.com> writes:\n\n> ... In the next few months, the Google\n> Git team is planning to implement some of the following changes, and\n> we'd appreciate your thoughts ahead of time as well as your review later\n> on:\n\nI was in some of the discussions that led to this message, and I\nfeel that some things need to be clarified.  The most important\nthing is that it is not a declaration that Google's Git team is\nsomehow trying to take the project maintenance or the review process\nover.  They are merely declaring what they will do to/on the list.\n\n> In the first half of January, I'd like to have a review out to the list\n> adding a kernel-style MAINTAINERS file with a few areas of interest. To\n> that file, I'd like to add \"R:\" (\"CC me!\") lines to subsystem mailing\n> lists and interested individuals. My hope is that that will make it easy\n> for someone to either add themselves as an \"R:\" or to subscribe to the\n> subsystem list, and then be able to filter their mail based on \"stuff\n> I'm CC'd to\" or \"stuff sent to git-partial-clone@linux.dev\" - and then\n> be able to review every patch in that list. I'd like to create lists on\n> topics where it makes sense, too.\n\nOK.  I expect that both the enumerations of \"areas of interest\" and\nthe parties who are interested in each area would be rather fluid,\nthough.  \"config\" might have somebody actively working on today, but\nmay not be in 3 months, and those who are interested in config today\nmay not be in 3 months.\n\nAlso, I'll mentally rephrase your \"review\" to \"reviewable patch\" in\nmy head from here on.\n\n>   Submodules\n>   submodule.[ch], git-submodule.sh, builtin/submodule.c,\n>     Documentation/git-submodule.txt, anything else adding a feature involving\n>     submodules\n>   R: git-submodules@example.com\n>   R: emilyshaffer@google.com\n\nI am afraid not many people on this list knows about the convention.\nThey can probably guess \"R:\" is \"CC me!\" from the previous\nparagraph, but they cannot guess (at least I can't) what it means to\nhave a mailing list there.  Does it mean a patch or a discussion\ndoes not have to happen here as long as it is done on a \"list\"\nelsewhere?\n\nAlso, the fluid-ness of the area might not work in favor of separate\nmailing lists.  We'll see.\n\n> 2. Draft a ReviewingPatches doc\n> Another theme we discussed was the general ambiguity around the act of\n> performing code review. How detailed should the review be? Who should be\n> examining interoperability with the rest of the codebase? Is it OK to\n> reply with only \"I don't know what you're trying to do, rewrite your\n> commit message please\"?\n>\n> Sometime in January I'd like to have a review out with an outline with\n> agreement on the content and organization for a ReviewingPatches doc.\n> I'm hoping that it can give some structure to review by including:\n>   - How to give different types of review (maintainer vs. individual vs.\n>     drive-by nits)\n\nIt is unclear what \"maintainer\" and \"individual\" reviews are;\nhopefully we see a clear definition in the January patch.\n\n> 3. Google Git team will review cover letters and commit messages\n>    internally before sending series to the list\n> I often find that when I send a patch to the list, reviewers reply\n> without understanding the point of what I was trying to achieve in the\n> patch. It sounded like this happens to some other Google Gitters as\n> well. Luckily, that's fixable by the patch author.\n>\n> To try and make sure we're sending patches that are easy to understand\n> and decide whether to review, we're adding a step to our process ASAP to\n> require commit messages, cover letters, and \"---\" notes to be reviewed\n> and approved by at least one other Googler before a topic goes to the\n> mailing list. Those reviews don't need to be formal, but do need to\n> happen for every reroll of a series. We also will intentionally *not*\n> review the diff of changes in this private setting - reviews for code\n> correctness, etc. should continue to happen upstream, in public.\n>\n> I'm mentioning that takeaway in this email to say: if you find you don't\n> understand the purpose of a patch from a Googler, please let us know! It\n> would be really valuable for us to receive a review saying \"I think you\n> might be saying X but it's still confusing because you wrote Y\"\n> so that we can incorporate the source of your confusion as we continue\n> to review each other's \"accompanying context\".\n\nThe reason behind this is because these Googler patch authors in the\ndiscussion feel that an initial \"why is this even needed?\" response\nhighly discouraging.  They are trying to, by proofreading the cover\nletters and log messages among themselves before they are sent, make\nsure that the motivation is clearly expressed in their patches.\n\nI think it is a good idea and would encourage everybody to follow,\nif possible.  Find a good sounding board in your buddies and ask\nthem to see if your cover letter and proposed log messages clearly\nconvey what you want to achieve and why it is a good thing, even to\nthose who do not necessarily share as much background context.  For\na solo developer (which I've been on this project for quite some\ntime), well, just \"sleeping on\" your patches often helps, as a\nrenewed you tomorrow will give you a different perspective.\n\nBut don't spend too much time on it to waste your momentum.  Having\nsounding board in your buddies is much better than not having any,\nbut your buddies may already share too much background context with\nyou to blind them from noticing why it is unclear to outsiders.\n\n> 4. Documentation changes to encourage commit message quality\n> ...\n> But I'd like to make sure that whatever commits we use as an example are\n> volunteered by the patch author, rather than chosen to be some example\n> of an antipattern. So if anybody wants to volunteer an example of a\n> particularly unclear commit that they wrote, that would be incredibly\n> useful.\n\nIf we can do so by picking good and bad example by the same author,\nthat would make a good educational experience to the readers.  Knowing\nthat nobody is consistently great would make it less stressful than\nalways having to try to be perfect.\n\n"},{"id":"444385","messageId":"20211217183942.npvkb3ajnx6p5cbp@meerkat.local","threadId":"57095","inReplyTo":"YbvBvch8JcHED+A9@google.com","subject":"Re: Review process improvements","fromName":"Konstantin Ryabitsev","fromEmail":"konstantin@linuxfoundation.org","sentAt":"2021-12-17T18:39:42Z","receivedAt":"2021-12-17T18:39:47Z","isPatch":false,"sender":{"key":"konstantin@linuxfoundation.org","avatar":"https://gravatar.com/avatar/7cb8827c6de56e1bd2dea16508c6708aa43feed3bf3813bcdacecdf96ceadd79?d=mp&s=160"},"body":"On Thu, Dec 16, 2021 at 02:46:21PM -0800, Emily Shaffer wrote:\n> Some of those discussions resulted in changes - for example,\n> GitGitGadget was improved and added to git/git, and we enjoy easy,\n> non-noisy CI runs via GitHub Actions. But some things we thought were a\n> good idea never went into practice. In the next few months, the Google\n> Git team is planning to implement some of the following changes, and\n> we'd appreciate your thoughts ahead of time as well as your review later\n> on:\n\nI'd like to also mention that I'm hoping to add a few more features to b4 that\nwill hopefully improve the patch submission process for folks. This feature\nset is far from being complete yet, but the gist will be as follows:\n\n1. b4 submit --new: this will create a new tracking branch and define\n   some metadata to go with it, such as a cover letter template. The cover\n   letter can be edited using `b4 submit --edit-cover` at any time.\n\n2. b4 submit --send: will generate a patch series from any commits created\n   from the topical branch fork point and use the cover letter from the\n   previous step. It will be able to send the patches using the traditional\n   SMTP way, OR it will allow using a web-based submission service I'm setting\n   up at kernel.org:\n\n   - anyone will be able to submit valid patches through this service\n   - one-off patch submissions will require an email confirmation roundtrip\n   - any submitter can also register their ed25519/openssh patatt key with the\n     submission service and just cryptographically sign their patches. As long\n     as signatures validate, no roundtrip confirmation of patches will be\n     required after the initial public key registration.\n   - on the receiving end, the patches will be written to a dedicated\n     lore.kernel.org feed *as-is*, but also sent to the recipients after doing\n     the usual From/Reply-To substitution and moving the original From into\n     the in-body git headers (i.e. same as GGG does).\n\n3. b4 submit will properly include base-commit information to all submitted\n   patches, as well as a unique change-id trailer (but in the basement of the\n   cover letter, not in the commit message trailers as Gerrit does).\n\n4. b4 submit --sync: will retrieve any received code review trailers\n   (reviewed-by, acked-by, etc) and amend the corresponding commits in the\n   topical branch, assuming we can match patch-id's (I've not tackled this\n   yet, so I may be unduly optimistic here).\n\n5. b4 submit --reroll: will prepare a v2 (v3, v4) of the series, reusing the\n   same change-id trailer and adding a templated \"Changes in v2\" entry to the\n   cover letter (that must be edited before --send works again).\n\n6. b4 submit --send: will send the new version of the series.\n\nI'm hoping that this will improve the experience of patch submitters *and*\nhelp ensure that CI can be incorporated into the process by streamlining the\nsubmission procedure and providing a public-inbox feed that can be easily\nmonitored. A service like GGG can fairly easily convert well-formed patch\nseries (at least those carrying valid base-commit info) into ephemeral\nworktrees, or even into simulated pull requests if the goal is to do this via\na generic CI service.\n\nThe important goal is to keep development fully decentralized so that even if\nthe web submission endpoint provided by kernel.org becomes unavailable, plain\nold SMTP submission mechanisms can still be used as a fallback.\n\n(Unfortunately, I need to focus my efforts on some kernel.org infrastructure\nchanges at this time, so I'm not sure when I'll be able to complete these\nfeatures.)\n\n> 1. Draft a MAINTAINERS file\n\nSo, I *don't* recommend that you go this route. The biggest problem with the\nMAINTAINERS file as used by the Linux development community is that making\nchanges to it is a very high-latency process. It will be less of a problem for\nthe much smaller git developer community, but it will still result in folks\noperating from a stale copy of the file for weeks after it is updated\nupstream.\n\nOne of the goals behind the \"lei\" tool by public-inbox is that it allows\nsearch-based subscriptions (including things like \"what files are being\nmodified, what functions are mentioned in the git context\", etc). Anyone can\ndefine a set of parameters they are interested in monitoring and receive only\nthreads matching those pre-filtered messages. We are currently rolling this\nout as an end-user maintained setup [1], but the goal is to also offer this to\nmaintainers as a centrally managed service:\n\n- maintainers will be able to define the search queries they are interested in\n  monitoring\n- we will set up a feed on our end matching those queries\n- we will offer the feed as a separate public-inbox repository, a public IMAP\n  folder, a read-only mailing list, and, I'm hoping, a read-only POP box (the\n  latter mostly so it can be configured to feed into GMail).\n\nThe hope is that this will allow what you're looking for -- a way to\npre-filter the flow of patches to maintainers by making it topical, without\nmaking the lives of patch submitters unnecessarily difficult by increasing the\nchances that they will send their patches to the wrong list.\n\n[1] https://people.kernel.org/monsieuricon/lore-lei-part-1-getting-started\n\nI'm not sure where this all fits into your plans, but I wanted to show what is\nhappening on our end just so we're working in parallel, as opposed to in our\nown separate worlds. :)\n\nBest regards,\n-K\n"},{"id":"444448","messageId":"CAP8UFD1M7LM630iHXb3EhgiRSoHgKzixTHmKYTA5OU1fEo69DA@mail.gmail.com","threadId":"57095","inReplyTo":"20211217183942.npvkb3ajnx6p5cbp@meerkat.local","subject":"Re: Review process improvements","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2021-12-20T10:48:24Z","receivedAt":"2021-12-20T10:48:40Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Dec 17, 2021 at 11:00 PM Konstantin Ryabitsev\n<konstantin@linuxfoundation.org> wrote:\n>\n> On Thu, Dec 16, 2021 at 02:46:21PM -0800, Emily Shaffer wrote:\n> > Some of those discussions resulted in changes - for example,\n> > GitGitGadget was improved and added to git/git, and we enjoy easy,\n> > non-noisy CI runs via GitHub Actions. But some things we thought were a\n> > good idea never went into practice. In the next few months, the Google\n> > Git team is planning to implement some of the following changes,\n\nThanks for trying to make it easier and more efficient to contribute!\n\n> > and\n> > we'd appreciate your thoughts ahead of time as well as your review later\n> > on:\n>\n> I'd like to also mention that I'm hoping to add a few more features to b4 that\n> will hopefully improve the patch submission process for folks.\n\nThanks also for implementing and maintaining tools and sites that help\ncontributing!\n\nI have added a new \"Process related tools and sites\" section to\nhttps://git.github.io/Hacking-Git/ with links to GitGitGadget,\nlore.kernel.org/git, public-inbox, lore+lei, b4 and git-series. I am\nwilling to add other tools and sites if someone knows others worth\nmentioning.\n\n[...]\n\n> > 1. Draft a MAINTAINERS file\n>\n> So, I *don't* recommend that you go this route. The biggest problem with the\n> MAINTAINERS file as used by the Linux development community is that making\n> changes to it is a very high-latency process. It will be less of a problem for\n> the much smaller git developer community, but it will still result in folks\n> operating from a stale copy of the file for weeks after it is updated\n> upstream.\n\nMy opinion is that a MAINTAINERS file makes more sense when there are\ndifferent mailing lists to send patches to, and trusted\nlieutenants/maintainers. I am not sure that we are at that point yet.\n\nAbout documentation, if some good documentation exists elsewhere, it\nmight be good enough to just point to it from an existing doc and/or\nhttps://git.github.io/Hacking-Git/. Otherwise adding a new separate\ndoc might be better than growing an existing doc too much, which could\nscare new comers.\n"},{"id":"444451","messageId":"211220.86fsqnwkzs.gmgdl@evledraar.gmail.com","threadId":"57095","inReplyTo":"YbvBvch8JcHED+A9@google.com","subject":"Re: Review process improvements","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-20T11:22:32Z","receivedAt":"2021-12-20T11:53:32Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Dec 16 2021, Emily Shaffer wrote:\n\n> In the first half of January, I'd like to have a review out to the list\n> adding a kernel-style MAINTAINERS file with a few areas of interest. To\n> that file, I'd like to add \"R:\" (\"CC me!\") lines to subsystem mailing\n> lists and interested individuals. My hope is that that will make it easy\n> for someone to either add themselves as an \"R:\" or to subscribe to the\n> subsystem list, and then be able to filter their mail based on \"stuff\n> I'm CC'd to\" or \"stuff sent to git-partial-clone@linux.dev\" - and then\n> be able to review every patch in that list. I'd like to create lists on\n> topics where it makes sense, too.\n>\n> Since the Git codebase isn't modularized into subsystems as plainly as\n> the kernel is, I don't think that the MAINTAINERS list needs to be\n> machine-parseable yet, although it would be a nice goal. Maybe over time\n> we will decide we'd rather organize the codebase differently and\n> implement a machine-parseable MAINTAINERS list to incorporate into a\n> sendemail-validate hook, or something. So I envision lines looking\n> something like this (just examples, sorry for directly calling people\n> out ;) ):\n>\n>   Submodules\n>   submodule.[ch], git-submodule.sh, builtin/submodule.c,\n>     Documentation/git-submodule.txt, anything else adding a feature involving\n>     submodules\n>   R: git-submodules@example.com\n>   R: emilyshaffer@google.com\n>\n>   Config improvements\n>   Documentation/config/*, config.[ch], builtin/config.c, changes which add new\n>     config options\n>   R: emilyshaffer@google.com\n>   R: avarab@gmail.com\n>\n> It would be extremely useful to hear other suggestions from the mailing\n> list about subsystems which deserve a MAINTAINERS line and possibly a\n> subsystem mailing list.\n\nKonstantin commented on some of this, and not to pile on, but I'd also\nnot like to see such a MAINTAINERS file in git.git for those sorts of\nthings.\n\nI think this makes sense for linux.git as different subsystems have\ndifferent mailing lists that are used for patch review, coordination\netc. Linus then pulls from those and other subsystem maintainers. There\nseem to be on the order of 100 of those split-outs now:\n\n    $ git grep -h -o linux-.*@.*kernel.org MAINTAINERS|sort -u|wc -l\n    77\n\nWhereas the only cases that really applies to in git.git (and I think it\nmight be useful to have a MAINTAINERS for these) are:\n\n    # but not all of po/, see caveats in po/README etc.\n    po/ -> https://github.com/git-l10n/git-po/\n    # Pulled from their respective upstreams\n    {gitweb,git-gui}/\n    # Anyhing else I'm missing? Maybe {sha1dc,sha1collisiondetection}/*?\n    \nBut (and I know you just used me as an example, but it applies in\ngeneral) I'd really not like to be CC'd on any change to config.[ch]\njust because I touched some code in those files that's clearly unrelated\nto whatever change is now being proposed.\n\nWe should be leaning into helper scripts like\ncontrib/contacts/git-contacts (and I'm aware of some out-of-tree \"better\ngit-contacts\", but have not used them myself).\n\nI.e. we almost always have a better and more accurate version of this\ninformation in the commit history.\n\nE.g. in the side-thread Randall asked to be CC'd on NonStop changes. I\nthink he should (and as does he), but I really don't see how it's\nnecessary to have a MAINTAINERS for such use-cases. If I'm editing what\nlittle NonStop-specific code we have in tree, it should be impossible to\nmiss Randall as an interested party when findings someone to CC through\nthe usual means.\n\nWhat are those \"usual means\"? I think they're different for most list\nregulars, but my personal worklow is (and I think some version of this\napplies to most regulars):\n\n1. As I'm preparing a patch series I'll need to run some of \"git log\",\n   \"git blame\", \"git log -G<relevant-string>\", \"git log -L:<funcname>:<file>\"\n   etc. just to understand the code I'm modifying.\n\n   As I'm doing that I take notes (usually these make it into draft commit\n   messages, noting prominent commits whose behavior is being changed). The\n   authors likewise make the draft CC list.\n\n   This is basically an approximation of what a script like \"git-contacts\"\n   tries to do, but I manually filter it. So e.g. I won't CC a person who\n   just tree-wide renamed a function I'm changing, even if that's the last\n   edit to the exact line I'm changing.\n\n2. I apply (sometimes on the fly) some fuzzy filtering to prune the above list.\n   Mostly this is to remove people from CC that I know to be inactive/gone, but\n   I'll also lean towards removing people I know to be CC'd a lot already, unless\n   the change really is highly relevant to them in particular.\n\nThis is manual for me now through a combination of not having bothered\nto automate it, and from having peeked a bit at some of automated\ntooling and concluded \"it looks like I'd need to manually filter this\nanyway\". E.g. I think \"git contacts\" can produce some rather naïve\noutput.\n\nBut leaning into that approach would be a much better\ndirection. E.g. the \"filter out inactive\" could be mostly/entirely done\nvia some configured heuristic in such a tool, together with downloading\nthe LKML archive to see how long it's been since someone last posted\nsomething.\n\nI can also imagine a MAINTAINERS being very useful for cases that aren't\nevident from scouring the commit logs, or for which it's nice to be\nexplicit about.\n\nE.g. I've often chimed in on changes to do withn how/whether something\ncan/should be marked for translation. I added the i18n subsystem\ninitiall, so for those cases interested parties would probably find me\nanyway, but I wouldn't mind being explicit about that. Ditto for people\nwe consider the authority on certain subsystems, such as (this may not\nbe any one person currently), refs, the index, SHA1<->SHA256 interop\netc.\n"},{"id":"444454","messageId":"YcB3LtxkItDVa2zd@sirena.org.uk","threadId":"57095","inReplyTo":"20211217183942.npvkb3ajnx6p5cbp@meerkat.local","subject":"Re: Review process improvements","fromName":"Mark Brown","fromEmail":"broonie@kernel.org","sentAt":"2021-12-20T12:29:34Z","receivedAt":"2021-12-20T12:29:42Z","isPatch":false,"sender":{"key":"broonie@kernel.org","avatar":null},"body":"On Fri, Dec 17, 2021 at 01:39:42PM -0500, Konstantin Ryabitsev wrote:\n\n> 2. b4 submit --send: will generate a patch series from any commits created\n>    from the topical branch fork point and use the cover letter from the\n>    previous step. It will be able to send the patches using the traditional\n>    SMTP way, OR it will allow using a web-based submission service I'm setting\n>    up at kernel.org:\n\nOne thing my equivalent of this does (I might've used patman but I don't\nthink I was aware of it at the time) is to tag each revision as it's\npublished, might be worth considering.\n"},{"id":"444505","messageId":"CAPig+cT+NshUMS+BNLKDMuwOEXKEzS3Ve=hK3kCYEBvJNzXuxQ@mail.gmail.com","threadId":"57095","inReplyTo":"211220.86fsqnwkzs.gmgdl@evledraar.gmail.com","subject":"Re: Review process improvements","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-20T17:14:02Z","receivedAt":"2021-12-20T17:14:15Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Dec 20, 2021 at 7:27 AM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> Whereas the only cases that really applies to in git.git (and I think it\n> might be useful to have a MAINTAINERS for these) are:\n>\n>     # but not all of po/, see caveats in po/README etc.\n>     po/ -> https://github.com/git-l10n/git-po/\n>     # Pulled from their respective upstreams\n>     {gitweb,git-gui}/\n>     # Anyhing else I'm missing? Maybe {sha1dc,sha1collisiondetection}/*?\n>\n> We should be leaning into helper scripts like\n> contrib/contacts/git-contacts (and I'm aware of some out-of-tree \"better\n> git-contacts\", but have not used them myself).\n\nFor completeness, the granddaddy of out-of-tree \"better\" tools is\nFelipe's git-related[1], which is maintained and more functional.\n\n[1]: https://github.com/felipec/git-related\n"},{"id":"444544","messageId":"J3wlC13aBBawF42_jmIX-9_4S5yvG4W-8miLgPASeby-v_QHn5Vw72gGy8E8WB-TQhGvrvpeC4Y2PnTIy3b1o6In_WHDzZ3s9nf2getOzRU=@protonmail.com","threadId":"57095","inReplyTo":"CAPig+cT+NshUMS+BNLKDMuwOEXKEzS3Ve=hK3kCYEBvJNzXuxQ@mail.gmail.com","subject":"Re: Review process improvements","fromName":"João Victor Bonfim","fromEmail":"joaovictorbonfim+git-mail-list@protonmail.com","sentAt":"2021-12-20T21:27:41Z","receivedAt":"2021-12-20T21:27:46Z","isPatch":false,"sender":{"key":"joaovictorbonfim+git-mail-list@protonmail.com","avatar":null},"body":"I added on to this on another e-mail thread, however, it got no response, specially because I didn't have an e-mail to reply to, so I am copy and pasting the messages here.\n\n‐‐‐‐‐‐‐ E-mail one ‐‐‐‐‐‐‐\n First and foremost, there is a saying in Brazil that goes by as \"an empty bag never holds itself standing\", so always remember to care for yourself, eat a snack, drink fluids of your choice (from tea, water and coffee to soup and porridge), rest a little, stop doom scrolling social media and so on.\n\n Second, want to address the giant essay by Emily Shaffer (aka: Review process improvements) and give my opinions on some of the points made. Third, sorry for the botched response, but I wasn't on the mailing list before the mail was posted, so couldn't directly respond the message.\n\n Addressing point #1, titled \"Draft a MAINTAINERS file\": seems quite reasonable, I would also like to address some matters about this, first is that there is no point of contact for \"trusted sources\" within the git project and it is quite negative for historical purposes, because maintainers and more prolific parties will inevitably retire or move on from Git themselves and their prestige won't be recorded beyond their commit history within the project and it might lead to their contributions being forgotten. Second is that it is hard to know who is responsible to what from the get go without being in the know already. Third, please make all entries on any and such file that might auto send messages non-committal, why? Nobody wants to receive a message from a third party that the git mailing list deems \"trustwothy\" only for it to be some scam of sorts that only happened because a modification to the file managed to fly under the radar, so make it a one way pass and all tags are only to people, not from. Fourth, I, personally, only want to partake on discussions, but barely want to see patches and commits, maybe allow for some sot of group inheritance and group message allow or deny lists? So people that don't want patches don't receive patches, but they can filter to receive only \"memory allocation\" topics, but they won't receive patches that are for memory allocation, because the \"patches\" and \"discussion\" topics take higher system priority than the \"memory allocation\" topic, be it user by user or system wide. Maybe also auto-filter messages, even in a dumb way or in a sender dependant way.\n\n Addressing point #2, titled \"Draft a ReviewingPatches doc\", and point #3, titled \"Google Git team will review cover letters and commit messages internally before sending series to the list\": not much to say beyond, people, share your reviewing know how, including you Google folks, if you interpret the reviewing process as an algorithm, it would make sense that all mechanisms of human interaction and review to be shared as part of the code base. So please, Google people, share what and how you do your reviews. It is also a matter of security, if your review process isn't transparent, we can't know whether we can trust everything you provide, because you might be leaving or dismissing problems and it might fly under the radar or malicious action could be taken and, since the group as a whole is already trusted, some malicious code could be included, even if it is not explicitly so.\n\n Addressing point #4, titled \"Documentation changes to encourage commit message quality\": This isn't stressed enough in many Git tutorials and the like, and it should, the commit messages are for the journaling of changes, so you or third parties can understand the thought process behind changes and not feel like headless chickens running around a barn whenever you attempt to understand what has been done, maybe addressing it on the source code of git and its documentation might help address this topic.\n\n Extra notes: As a person with ADHD, REAMEs can be daunting at times, specially when they are boring word walls, and they can be incomplete or repetitive if there are other documents addressing information contained within them, maybe reference files such as \"Contributing.md\" instead of copying them verbatim? An example would be \"To read more on how to contribute to projects, read 'Contributing.md'.\" instead of what is information contained within \"Contributing.md\", it would help a lot with human to human interactions.\n\n Thanks for the attention, take care!\n\n João Victor Bonfim (any pronouns).\n‐‐‐‐‐‐‐ E-mail two ‐‐‐‐‐‐‐\nTo address Junio Hamano's comment:\n>  But don't spend too much time on it to waste your momentum.  Having\n> sounding board in your buddies is much better than not having any,\n> but your buddies may already share too much background context with\n> you to blind them from noticing why it is unclear to outsiders.\n\n Maybe ask someone like your mom, dad, aunt, cousin, long past college acquaintance to comment on the commit/log/patch comment itself and maybe also the code published? It might me more helpful the less they understand about code or are knowledgeable about your personal quirks.\n\nJoão Victor Bonfim (any pronouns).\n\n‐‐‐‐‐‐‐------------‐‐‐‐‐‐‐\n\nYes, I know I wrote an essay-esque message myself.\nIt was a lot.\nHope it helps to move the conversation along.\n\n‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐\n\nEm segunda-feira, 20 de dezembro de 2021 às 14:14, Eric Sunshine <sunshine@sunshineco.com> escreveu:\n\n> On Mon, Dec 20, 2021 at 7:27 AM Ævar Arnfjörð Bjarmason\n>\n> avarab@gmail.com wrote:\n>\n> > Whereas the only cases that really applies to in git.git (and I think it\n> >\n> > might be useful to have a MAINTAINERS for these) are:\n> >\n> >     # but not all of po/, see caveats in po/README etc.\n> >     po/ -> https://github.com/git-l10n/git-po/\n> >     # Pulled from their respective upstreams\n> >     {gitweb,git-gui}/\n> >     # Anyhing else I'm missing? Maybe {sha1dc,sha1collisiondetection}/*?\n> >\n> >\n> > We should be leaning into helper scripts like\n> >\n> > contrib/contacts/git-contacts (and I'm aware of some out-of-tree \"better\n> >\n> > git-contacts\", but have not used them myself).\n>\n> For completeness, the granddaddy of out-of-tree \"better\" tools is\n>\n> Felipe's git-related[1], which is maintained and more functional.\n>\n> [1]: https://github.com/felipec/git-related\n"},{"id":"444570","messageId":"YcEyLMkM3fvGYCUY@camp.crustytoothpaste.net","threadId":"57095","inReplyTo":"211220.86fsqnwkzs.gmgdl@evledraar.gmail.com","subject":"Re: Review process improvements","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-12-21T01:47:24Z","receivedAt":"2021-12-21T01:47:29Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-12-20 at 11:22:32, Ævar Arnfjörð Bjarmason wrote:\n> E.g. I've often chimed in on changes to do withn how/whether something\n> can/should be marked for translation. I added the i18n subsystem\n> initiall, so for those cases interested parties would probably find me\n> anyway, but I wouldn't mind being explicit about that. Ditto for people\n> we consider the authority on certain subsystems, such as (this may not\n> be any one person currently), refs, the index, SHA1<->SHA256 interop\n> etc.\n\nI have limited time for the list, but I also tend to pop in on certain\ntopics: the SHA-256 transition, commit and tag signing, authentication,\ntranslations, and so on.  However, outside of the signing code, a lot of\nthat isn't centralized in one place.\n\nI also wouldn't be able to commit to signing up for any subsystem in a\nMAINTAINERS file, but I'm happy to be CC'd if folks think I'm\nknowledgeable about something or if it's an area they think my opinion\nwould be valuable.\n\nI can't speak for others, though, so I don't know if a MAINTAINERS-style\napproach would be valuable for them.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"444704","messageId":"211222.86ee65pb60.gmgdl@evledraar.gmail.com","threadId":"57095","inReplyTo":"20211217183942.npvkb3ajnx6p5cbp@meerkat.local","subject":"Re: Review process improvements","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-22T03:26:39Z","receivedAt":"2021-12-22T03:32:11Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Dec 17 2021, Konstantin Ryabitsev wrote:\n\n> [...]\n>    - on the receiving end, the patches will be written to a dedicated\n>      lore.kernel.org feed *as-is*, but also sent to the recipients after doing\n>      the usual From/Reply-To substitution and moving the original From into\n>      the in-body git headers (i.e. same as GGG does).\n\nThat GGG does this is one reason I haven't considered using it. It\nbreaks all sorts of E-Mail workflow assumptions from polluting the\naddress book for every person who uses it, to any \"from:<addr>\" search\nneeding special consideration etc.\n\nOf course you'd need authentication etc, but is there a reason for why\nsuch tooling can't work more like an SMTP relay and less like GGG which\n(for some reason) insists on taking over the \"From\" header?\n\nI think in its case it's because it wanted to mirror all the discussion\nto GitHub. But presumably a ML-native tool won't have that problem (and\npresumably GGG could do the same mirroring by following the ML after\nsubmission).\n\n"},{"id":"444768","messageId":"20211222130744.4dmpa6joac2rc2vt@fs","threadId":"57095","inReplyTo":"211222.86ee65pb60.gmgdl@evledraar.gmail.com","subject":"Re: Review process improvements","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-12-22T13:07:44Z","receivedAt":"2021-12-22T13:07:49Z","isPatch":false,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 22.12.2021 04:26, Ævar Arnfjörð Bjarmason wrote:\n>\n>On Fri, Dec 17 2021, Konstantin Ryabitsev wrote:\n>\n>> [...]\n>>    - on the receiving end, the patches will be written to a dedicated\n>>      lore.kernel.org feed *as-is*, but also sent to the recipients after doing\n>>      the usual From/Reply-To substitution and moving the original From into\n>>      the in-body git headers (i.e. same as GGG does).\n>\n>That GGG does this is one reason I haven't considered using it. It\n>breaks all sorts of E-Mail workflow assumptions from polluting the\n>address book for every person who uses it, to any \"from:<addr>\" search\n>needing special consideration etc.\n\nI agree that this is a very annoying side-effect of GGG.\n\n>\n>Of course you'd need authentication etc, but is there a reason for why\n>such tooling can't work more like an SMTP relay and less like GGG which\n>(for some reason) insists on taking over the \"From\" header?\n>\n\nI think SPF/DMARC spam filtering will prevent tools like GGG to do this any \nother way. For GGG to be able to send with your From: you'd either need to \ngive it access to your smtp credentials or configure your whole domain to \nallow GGG mail servers (basically github) to send email for it. I think both \noptions are not really something you'd want.\n\n>I think in its case it's because it wanted to mirror all the discussion\n>to GitHub. But presumably a ML-native tool won't have that problem (and\n>presumably GGG could do the same mirroring by following the ML after\n>submission).\n>\n"},{"id":"444782","messageId":"20211222154534.b2gb22ghn2mpyeur@meerkat.local","threadId":"57095","inReplyTo":"211222.86ee65pb60.gmgdl@evledraar.gmail.com","subject":"Re: Review process improvements","fromName":"Konstantin Ryabitsev","fromEmail":"konstantin@linuxfoundation.org","sentAt":"2021-12-22T15:45:34Z","receivedAt":"2021-12-22T15:45:39Z","isPatch":false,"sender":{"key":"konstantin@linuxfoundation.org","avatar":"https://gravatar.com/avatar/7cb8827c6de56e1bd2dea16508c6708aa43feed3bf3813bcdacecdf96ceadd79?d=mp&s=160"},"body":"On Wed, Dec 22, 2021 at 04:26:39AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> That GGG does this is one reason I haven't considered using it. It\n> breaks all sorts of E-Mail workflow assumptions from polluting the\n> address book for every person who uses it, to any \"from:<addr>\" search\n> needing special consideration etc.\n> \n> Of course you'd need authentication etc, but is there a reason for why\n> such tooling can't work more like an SMTP relay and less like GGG which\n> (for some reason) insists on taking over the \"From\" header?\n\nThis would require pretending that we're authorized to send mail from the\ndomain name of the commit author, so this unfortunately won't work (and hence\nthe reason why GGG does it this way). E.g. say you have:\n\nFrom: foo@redhat.com\nSubject: [PATCH] Fix foo\n\nFor DMARC policies to properly work, this message would need to have a DKIM\nsignature from redhat.com, so we'd need to have access to Red Hat's private\nkeys. If we don't use DKIM signatures, then the recipient SMTP gateways may\nmark the message as spam (and they would be right, since we are pretending to\nbe foo@redhat.com, i.e. acting just like phishers).\n\nThe only way for this to work is to do the From / X-Original-From /\nFollow-up-to / Reply-To header dance. Git does support this very well, and\nmost email clients do the right thing when encountering this situation, but\nit's not going to have the exact same visual flow as patches sent directly via\nSMTP.\n\nHowever, we will also write these messages to a public-inbox repository before\nmaking the From: substitutions, so if someone retrieves these patches with b4\nor lei, as opposed to receiving them via their SMTP mailbox, they should be\nable to get proper From: in the headers.\n\n-K\n"},{"id":"444817","messageId":"xmqqh7b0juk5.fsf@gitster.g","threadId":"57095","inReplyTo":"20211222154534.b2gb22ghn2mpyeur@meerkat.local","subject":"Re: Review process improvements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-22T19:42:02Z","receivedAt":"2021-12-22T19:42:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:\n\n> This would require pretending that we're authorized to send mail from the\n> domain name of the commit author, so this unfortunately won't work (and hence\n> the reason why GGG does it this way). E.g. say you have:\n>\n> From: foo@redhat.com\n> Subject: [PATCH] Fix foo\n\nWould it help to use \"Sender:\"?  When GGG or any other automation\nare trying to send e-mail on behalf of the person shown on \"From:\",\nI thought that it is the mechanism for them to use to identify\nthemselves.\n\n\n"},{"id":"444826","messageId":"20211222213247.5dnj3zlj53lh6l32@meerkat.local","threadId":"57095","inReplyTo":"xmqqh7b0juk5.fsf@gitster.g","subject":"Re: Review process improvements","fromName":"Konstantin Ryabitsev","fromEmail":"konstantin@linuxfoundation.org","sentAt":"2021-12-22T21:32:47Z","receivedAt":"2021-12-22T21:32:52Z","isPatch":false,"sender":{"key":"konstantin@linuxfoundation.org","avatar":"https://gravatar.com/avatar/7cb8827c6de56e1bd2dea16508c6708aa43feed3bf3813bcdacecdf96ceadd79?d=mp&s=160"},"body":"On Wed, Dec 22, 2021 at 11:42:02AM -0800, Junio C Hamano wrote:\n> > This would require pretending that we're authorized to send mail from the\n> > domain name of the commit author, so this unfortunately won't work (and hence\n> > the reason why GGG does it this way). E.g. say you have:\n> >\n> > From: foo@redhat.com\n> > Subject: [PATCH] Fix foo\n> \n> Would it help to use \"Sender:\"?  When GGG or any other automation\n> are trying to send e-mail on behalf of the person shown on \"From:\",\n> I thought that it is the mechanism for them to use to identify\n> themselves.\n\nIndeed, that's how the DKIM standard wanted to deal with this problem, however\nwhen the DMARC RFC was being drafted, this approach was deemed insufficient.\nThey have a good explanation for it -- there is no standard among UI clients\nto handle the Sender/From discrepancy. Most MUAs will happily ignore the\nSender: field and will only show what is in From:, so this approach was\nconsidered ineffective against phishing attacks. An attacker could easily\nregister a domain, set DKIM records, and then use any From: they wanted as\nlong as they used a valid Sender: header, knowing that it would be ignored by\nmost mail clients.\n\nSo, DMARC deliberately ignores the Sender: header and *only* pays attention to\nthe From: field for its purpose.\n\n-K\n"},{"id":"444858","messageId":"YcR+rZ7YznQlTuRf@stefanha-x1.localdomain","threadId":"57095","inReplyTo":"20211217183942.npvkb3ajnx6p5cbp@meerkat.local","subject":"Re: Review process improvements","fromName":"Stefan Hajnoczi","fromEmail":"stefanha@redhat.com","sentAt":"2021-12-23T13:50:37Z","receivedAt":"2021-12-23T13:50:45Z","isPatch":false,"sender":{"key":"stefanha@redhat.com","avatar":null},"body":"> I'd like to also mention that I'm hoping to add a few more features to b4 that\n> will hopefully improve the patch submission process for folks.\n\nThis looks excellent! I wanted to mention a few features that have been\npopular in the git-publish patch series management tool\n(https://github.com/stefanha/git-publish/) in case you want to include\nsomething similar in b4:\n\n- \"Profiles\" (.gitpublishprofile or stored in .git/config) with To and\n  CC addresses, --subject-prefix, cover letter templates, etc. The\n  default profile is automatically loaded when you run \"git publish\". If\n  the git repo you're working on includes a default profile then you\n  don't need to specify any command-line options to send a correctly\n  formatted patch series to the maintainers! A great help for one-off\n  contributors but also a time-saver for regular contributors.\n\n- An interactive mode that lets you inspect the final patch series and\n  edit the CC list. Avoids mistakes and embarassment :).\n\n- Saving the CC list between revisions (v2, v3, etc).\n\n- Sending pull requests (for sub-maintainers) because there are times\n  when pull requests also need a v2 or even a v3 ;). The workflow is\n  basically the same as sending patch series.\n\n- githooks(5) before sending patches series and pull requests. Useful\n  for coding style and patch series linters.\n\nThe git-publish man page is here if you want to read more:\nhttps://github.com/stefanha/git-publish/blob/master/git-publish.pod#quickstart\n\nStefan\n"},{"id":"445495","messageId":"YdTqQDnHLY7oUTja@google.com","threadId":"57095","inReplyTo":"YbvBvch8JcHED+A9@google.com","subject":"Re: Review process improvements","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2022-01-05T00:45:52Z","receivedAt":"2022-01-05T00:46:00Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Thu, Dec 16, 2021 at 02:46:21PM -0800, Emily Shaffer wrote:\n\nReshuffling context....\n\n> As it's reaching the end of the year and the holiday season, I'm\n> planning to spend the rest of the year with family and away from my work\n> email as much as possible. Some other Googler Gitters will be working on\n> and off and may chime in. But I am expecting to be able to engage more\n> with this mail in the first week of January.\n\nHaving had a very restful couple of weeks off, I've now read through all\nthe replies to this mail - thank you! - and will summarize as much as I\ncan.\n\n> 1. Draft a MAINTAINERS file\n> One recurring theme from the conversations I had with the Google Git\n> team was that selecting a topic to review in the first place can\n> actually be pretty time-consuming. Choosing from the series subject line\n> or the \"What's cooking\" mail isn't always straightforward - it can be\n> hard to tell whether the series is working on a goal you are interested\n> in or in a part of the codebase you care about. It's not feasible for\n> someone wanting to get involved to review every single series that comes\n> across the list - but it might be feasible for someone to review every\n> series in a particular subsystem or topic.\n> \n> In the first half of January, I'd like to have a review out to the list\n> adding a kernel-style MAINTAINERS file with a few areas of interest. To\n> that file, I'd like to add \"R:\" (\"CC me!\") lines to subsystem mailing\n> lists and interested individuals. My hope is that that will make it easy\n> for someone to either add themselves as an \"R:\" or to subscribe to the\n> subsystem list, and then be able to filter their mail based on \"stuff\n> I'm CC'd to\" or \"stuff sent to git-partial-clone@linux.dev\" - and then\n> be able to review every patch in that list. I'd like to create lists on\n> topics where it makes sense, too.\n\nSummary: Everybody pretty much hated this idea. ;) I count at least five\nreplies saying \"please let's not, it's too cumbersome\".\n\nThat makes sense to me. As Ævar mentioned - just because he and I are\nworking on config a lot right now, doesn't mean we will still care in\n2023. And remembering to remove yourself from the MAINTAINERS file is a\npain.\n\nÆvar suggested leaning in harder to improving the tooling around \"who do\nI CC?\"\n(https://lore.kernel.org/git/211220.86fsqnwkzs.gmgdl%40evledraar.gmail.com)\nand I think that's a reasonable approach, if we pair it with updating\nSubmittingPatches and MyFirstContribution to include recommending\nrunning the \"who do I CC?\" script.\n\n> Since the Git codebase isn't modularized into subsystems as plainly as\n> the kernel is, I don't think that the MAINTAINERS list needs to be\n> machine-parseable yet, although it would be a nice goal. Maybe over time\n> we will decide we'd rather organize the codebase differently and\n> implement a machine-parseable MAINTAINERS list to incorporate into a\n> sendemail-validate hook, or something. So I envision lines looking\n> something like this (just examples, sorry for directly calling people\n> out ;) ):\n> \n>   Submodules\n>   submodule.[ch], git-submodule.sh, builtin/submodule.c,\n>     Documentation/git-submodule.txt, anything else adding a feature involving\n>     submodules\n>   R: git-submodules@example.com\n>   R: emilyshaffer@google.com\n> \n>   Config improvements\n>   Documentation/config/*, config.[ch], builtin/config.c, changes which add new\n>     config options\n>   R: emilyshaffer@google.com\n>   R: avarab@gmail.com\n> \n> It would be extremely useful to hear other suggestions from the mailing\n> list about subsystems which deserve a MAINTAINERS line and possibly a\n> subsystem mailing list.\n\nRandall suggested using a prefix/tagging approach to make it easy for\nreviewers to filter by topic. I think that's a pretty good idea! But I\nalso think for it to be useful as a filtering tool we'd want to\nformalize \"this topic goes to this tag\" - which brings us back along to\nthe cumbersome-ness of the MAINTAINERS file.\n\nI see a couple options:\n\n1) Eat the maintenance overhead of having a checked-in file mapping\nfilespec/topic to subject line tag, like the sample Randall gave\n(https://lore.kernel.org/git/00b901d7f2d3%24c0d2ca20%2442785e60%24%40nexbridge.com)\nbut minus the \"Reviewers:\" section:\n\nSubmodules:\n  Prefix: SUBM\n  Files:\n    submodule.[ch], git-submodule.sh, builtin/submodule.c,\n    Documentation/git-submodule.txt\nPlatform:\n  Linux:\n    Prefix: LINUX\n  NonStop:\n    Prefix: NS\n\nHaving these mails all still delivered to git@vger.kernel.org means that\nit's not such a big deal if the prefix becomes obsolete, and not\nincluding an individual CC list means that nobody needs to remove\nthemselves from CC list after they stop caring about some part of the\ncodebase.\n\nAn alternative to subject lines like \"Subject: SUBM: share config\nbetween blah blah\" might be possible. Some providers (at least Gmail,\nand based on João's reply-to address, protonmail as well) allow mails\naddressed to e.g. \"my-name+from-git-list@example.com\" to be delivered to\n\"my-name@example.com\" regardless of what follows the +; if vger allows\nthis kind of thing, we could save some visual space by adopting the\nconvention that e.g. submodule topics should go to\n\"git-subm@vger.kernel.org\" and allow folks to write their mail filters\naccordingly. (I took a poke through IETF RFC5322 to see if this +\ncomment/tag/whatever-it-is is standardized and didn't see anything\npromising, so maybe this alternative is a bad idea. Or maybe I didn't\nGoogle well enough.)\n\n2) Put that kind of mapping into an easier-to-modify place, like a wiki. The\ndownside is that, as far as I'm aware, we don't have or maintain an\nexisting git.git wiki, so it'd probably end up going unused as people\ndon't want to start looking at yet another discussion area.\n\n\n\nEither way, \"Check this file/page and determine what additional tag to\nadd to your outgoing patches\" would need to be called out in\ndocumentation like SubmittingPatches//MyFirstContribution - and modeled\nby veteran prolific contributors in their own patches.\n\nIf we're generally happy with the idea of tagging rather than additional\nlists, then I think there's nothing in the way of beginning to draft\nchanges to either of those docs in order to agree on the standard we\nwant to adopt. But I'd like to make sure that we are in agreement first.\nMaybe the best way to find that agreement is via the doc patches\nthemselves?\n\n> 2. Draft a ReviewingPatches doc\n> Another theme we discussed was the general ambiguity around the act of\n> performing code review. How detailed should the review be? Who should be\n> examining interoperability with the rest of the codebase? Is it OK to\n> reply with only \"I don't know what you're trying to do, rewrite your\n> commit message please\"?\n> \n> Sometime in January I'd like to have a review out with an outline with\n> agreement on the content and organization for a ReviewingPatches doc.\n> I'm hoping that it can give some structure to review by including:\n>   - How to give different types of review (maintainer vs. individual vs.\n>     drive-by nits)\n>   - Review best practices (Sage Sharp's article on stages of review[4],\n>     CCing experts/lists from MAINTAINERS file above, short-circuiting by\n>     sending comments early instead of reviewing a patch in-depth when\n>     the idea seems so-so or unclear)\n>   - Checklists for new contributors to use or to help make in-depth\n>     review more thorough\n> \n> Since this is intended to be checked into Git's codebase, I expect that\n> we'll want to discuss a lot and make sure we all buy into the contents\n> of that doc.\n\nThis idea seemed generally well-received. I noticed that João was\ncurious to hear more about Google's review process; I'll state that\nthere is less magic than you might think, and whatever we do know will\ncertainly make it into a draft.\n\nI think there is nothing standing in the way of sending out an outline\nand/or first draft of such a document for us to all discuss and quibble\nover. :)\n\n> \n> 3. Google Git team will review cover letters and commit messages\n>    internally before sending series to the list\n> I often find that when I send a patch to the list, reviewers reply\n> without understanding the point of what I was trying to achieve in the\n> patch. It sounded like this happens to some other Google Gitters as\n> well. Luckily, that's fixable by the patch author.\n> \n> To try and make sure we're sending patches that are easy to understand\n> and decide whether to review, we're adding a step to our process ASAP to\n> require commit messages, cover letters, and \"---\" notes to be reviewed\n> and approved by at least one other Googler before a topic goes to the\n> mailing list. Those reviews don't need to be formal, but do need to\n> happen for every reroll of a series. We also will intentionally *not*\n> review the diff of changes in this private setting - reviews for code\n> correctness, etc. should continue to happen upstream, in public.\n> \n> I'm mentioning that takeaway in this email to say: if you find you don't\n> understand the purpose of a patch from a Googler, please let us know! It\n> would be really valuable for us to receive a review saying \"I think you\n> might be saying X but it's still confusing because you wrote Y\"\n> so that we can incorporate the source of your confusion as we continue\n> to review each other's \"accompanying context\".\n\nIt seems that folks liked this idea, and furthermore, that it would be\nworth suggesting such a process - \"ask your friend to read your commit\nmessage!\" - in one or both of SubmittingPatches and MyFirstContribution.\n\n> \n> 4. Documentation changes to encourage commit message quality\n\nI think I didn't hear much response to this other than \"sure, sounds\nfine\" - so I expect that with the changes themselves will come more\ndetailed discussion. I also don't think there is anything stopping these\nchanges from happening.\n\n---\n\nIn general, I think everybody seems enough on board for us to begin\nsending doc patches. So the next step for me is to work out which of we\nGooglers will handle which, and for us to start sending stuff for\nreview. Thank you to everybody who contributed on this thread and thanks\nin advance for reviews on the patches to come.\n\n - Emily\n"},{"id":"445497","messageId":"YdTuMlAkVIivp1bg@google.com","threadId":"57095","inReplyTo":"J3wlC13aBBawF42_jmIX-9_4S5yvG4W-8miLgPASeby-v_QHn5Vw72gGy8E8WB-TQhGvrvpeC4Y2PnTIy3b1o6In_WHDzZ3s9nf2getOzRU=@protonmail.com","subject":"Re: Review process improvements","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2022-01-05T01:02:42Z","receivedAt":"2022-01-05T01:02:49Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Mon, Dec 20, 2021 at 09:27:41PM +0000, João Victor Bonfim wrote:\n> \n> I added on to this on another e-mail thread, however, it got no\n> response, specially because I didn't have an e-mail to reply to, so I\n> am copy and pasting the messages here.\n\nSince you mentioned not receiving a reply earlier, I thought I would\nreply directly to your comments.\n\nI have only recently seen you begin to post here, so welcome! and I\nthink I saw someone else mention in another thread, but I'll say again\nnow: in general, please wrap your lines at ~80ch when replying to the\nmailing list; having very very long lines like the ones you sent is\nannoying for some mail clients, so the Git list convention is to wrap\nthem. I rewrapped your mail below so that it would fit more easily into\nmy editor.\n\n> \n> ‐‐‐‐‐‐‐ E-mail one ‐‐‐‐‐‐‐\n>  Addressing point #1, titled \"Draft a MAINTAINERS file\": seems quite\n>  reasonable, I would also like to address some matters about this,\n>  first is that there is no point of contact for \"trusted sources\"\n>  within the git project and it is quite negative for historical\n>  purposes, because maintainers and more prolific parties will\n>  inevitably retire or move on from Git themselves and their prestige\n>  won't be recorded beyond their commit history within the project and\n>  it might lead to their contributions being forgotten. Second is that\n>  it is hard to know who is responsible to what from the get go without\n>  being in the know already. Third, please make all entries on any and\n>  such file that might auto send messages non-committal, why? Nobody\n>  wants to receive a message from a third party that the git mailing\n>  list deems \"trustwothy\" only for it to be some scam of sorts that\n>  only happened because a modification to the file managed to fly under\n>  the radar, so make it a one way pass and all tags are only to people,\n>  not from.\n\nI'm not really sure what you mean by \"non-committal\" here. I will say\nthat messages coming from the Git mailing list does not imply that\nmessages are safe in any way; we get plenty of spam and phishing mails\nmaking it past vger.kernel.org's filters. The proposal of a MAINTAINERS\nfile was definitely not a proposal to modify the review process itself;\nas always, the expectation is that code should be reviewed by a number\nof contributors to ensure it's doing what it says it is trying to do. I\ndon't see that that will ever change.\n\n>  Fourth, I, personally, only want to partake on discussions,\n>  but barely want to see patches and commits, maybe allow for some sot\n>  of group inheritance and group message allow or deny lists? So people\n>  that don't want patches don't receive patches, but they can filter to\n>  receive only \"memory allocation\" topics, but they won't receive\n>  patches that are for memory allocation, because the \"patches\" and\n>  \"discussion\" topics take higher system priority than the \"memory\n>  allocation\" topic, be it user by user or system wide. Maybe also\n>  auto-filter messages, even in a dumb way or in a sender dependant\n>  way.\n\nFor what it's worth, in my Gmail web client I filter out any mails\nbeginning with `[PATCH` - because in web client I am like you and\nusually only want to participate in broader discussions. So I think\nthere is already a solution for filtering the way that you want to.\n\n> \n>  Addressing point #2, titled \"Draft a ReviewingPatches doc\", and point\n>  #3, titled \"Google Git team will review cover letters and commit\n>  messages internally before sending series to the list\": not much to\n>  say beyond, people, share your reviewing know how, including you\n>  Google folks, if you interpret the reviewing process as an algorithm,\n>  it would make sense that all mechanisms of human interaction and\n>  review to be shared as part of the code base. So please, Google\n>  people, share what and how you do your reviews. It is also a matter\n>  of security, if your review process isn't transparent, we can't know\n>  whether we can trust everything you provide, because you might be\n>  leaving or dismissing problems and it might fly under the radar or\n>  malicious action could be taken and, since the group as a whole is\n>  already trusted, some malicious code could be included, even if it is\n>  not explicitly so.\n\nAs I mentioned in my mail, we are only conducting review of commit\nmessages and cover letters, not of code - specifically *because* it is\nso important to perform in-depth code review out in the open, for the\nreasons you say. Code review should always happen on this list, and I'm\nnot suggesting to change that. (That's true of patches submitted via\nGitGitGadget too, by the way - we don't perform review in the comments\non those GGG pull requests, for these same reasons.)\n\nAs for the Googley review know-how.... like I mentioned in my reply to\nthe main thread a moment ago, there's not that much \"secret sauce\". :)\nHowever, if you're curious, you might keep an eye on the #git-devel\nchannel - we have recently started doing public \"review club\" live\nmeetings and inviting the Git community to join us in reviewing patches\non the Git list. The next one is this week, so if you're interested and\nthe timezone works out, you'd be welcome to join us.\n\n> \n>  Extra notes: As a person with ADHD, REAMEs can be daunting at times,\n>  specially when they are boring word walls, and they can be incomplete\n>  or repetitive if there are other documents addressing information\n>  contained within them, maybe reference files such as\n>  \"Contributing.md\" instead of copying them verbatim? An example would\n>  be \"To read more on how to contribute to projects, read\n>  'Contributing.md'.\" instead of what is information contained within\n>  \"Contributing.md\", it would help a lot with human to human\n>  interactions.\n\nYes, I agree this is the best way to do documentation. Human ability to\nparse or no - having the same information in two places means that\ninevitably, one place will become stale. ;)\n\n>  Thanks for the attention, take care!\n\nAgain, welcome to the project.\n\n - Emily\n"},{"id":"445799","messageId":"rcGd-SpUU-W-tTnksrZa3gaeC9Kswpy7oPiOyp-GGV5WJgkoICgiD5ppeFz3h89h0l8C1fSRv5XcI6pqR4uEaG8mh29uWmPUDk-NORRkt2o=@protonmail.com","threadId":"57095","inReplyTo":"YdTuMlAkVIivp1bg@google.com","subject":"Re: Review process improvements","fromName":"João Victor Bonfim","fromEmail":"joaovictorbonfim+git-mail-list@protonmail.com","sentAt":"2022-01-09T03:26:14Z","receivedAt":"2022-01-09T03:26:20Z","isPatch":false,"sender":{"key":"joaovictorbonfim+git-mail-list@protonmail.com","avatar":null},"body":"> That makes sense to me. As Ævar mentioned - just because he and I are\n> working on config a lot right now, doesn't mean we will still care in\n> 2023. And remembering to remove yourself from the MAINTAINERS file is a\n> pain.\n\n Maybe make a \"contributors\" file instead and merely list there who have\ncontributed with Git, what have they contributed with (or group\ncontributors by their types of contribution) and means of contact, like\ninstitutions they are a part of, e-mail addresses, telephone numbers,\nwhile highlighting that all e-mails related to Git are expected to be\nshared with the mailing list unless stated otherwise. Also make the\ninput of personal information beyond name voluntary.\n\n> Some providers (at least Gmail, and based on João's reply-to address,\n> protonmail as well) allow mails addressed to\n> e.g. \"my-name+from-git-list@example.com\" to be delivered to\n> \"my-name@example.com\" regardless of what follows the +;\n\n Although ProtonMail allows the use of + aliases, no filtering\nis done automatically.\n\n> I have only recently seen you begin to post here, so welcome! and I\n> think I saw someone else mention in another thread, but I'll say again\n> now: in general, please wrap your lines at ~80ch when replying to the\n> mailing list;\n\n Is \"80 ch\" 80 American units? (half-joke)\n I also have no idea on how to configure that.\n\n> I'm not really sure what you mean by \"non-committal\" here. I will say\n> that messages coming from the Git mailing list does not imply that\n> messages are safe in any way; we get plenty of spam and phishing mails\n> making it past vger.kernel.org's filters. The proposal of a MAINTAINERS\n> file was definitely not a proposal to modify the review process itself;\n> as always, the expectation is that code should be reviewed by a number\n> of contributors to ensure it's doing what it says it is trying to do. I\n> don't see that that will ever change.\n\n Brain fog, I don't exactly remember either. From what I got I was\nthinking about the servers sending messages in the name of the parties\nlisted, not only to them. Also I might be thinking of people sending\nmalicious code through commits and git operations and it causing\nproblems and also about e-mail addresses being compromised.\n\n> For what it's worth, in my Gmail web client I filter out any mails\n> beginning with `[PATCH` - because in web client I am like you and\n> usually only want to participate in broader discussions. So I think\n> there is already a solution for filtering the way that you want to.\n\n Can't we automate it at the source, though...\n ;A;\n\n> As I mentioned in my mail, we are only conducting review of commit\n> messages and cover letters, not of code - specifically because it is\n> so important to perform in-depth code review out in the open, for the\n> reasons you say. Code review should always happen on this list, and I'm\n> not suggesting to change that. (That's true of patches submitted via\n> GitGitGadget too, by the way - we don't perform review in the comments\n> on those GGG pull requests, for these same reasons.)\n>\n> As for the Googley review know-how.... like I mentioned in my reply to\n> the main thread a moment ago, there's not that much \"secret sauce\". :)\n\n I still believe that methods of reviewing commit messages might be\nhelpful if shared collectively.\n\n> However, if you're curious, you might keep an eye on the #git-devel\n> channel - we have recently started doing public \"review club\" live\n> meetings and inviting the Git community to join us in reviewing patches\n> on the Git list. The next one is this week, so if you're interested and\n> the timezone works out, you'd be welcome to join us.\n\n I both don't know what is Devel and how to use it and don't believe\nanxiety will allow me, specially since I am very socially anxious, I\nam using a TV screen as a \"temporary\" monitor and it triggers my\nsensory sensitivities, because of the amount of information and light\n(having a text heavy profession might have been a bad idea), and...\n\n*panic*\n\n... anxiety??? (better not delve too deep into that can of worms)\n\n> Yes, I agree this is the best way to do documentation. Human ability to\n> parse or no - having the same information in two places means that\n> inevitably, one place will become stale. ;)\n\n Maybe use <iframe>s to embed files onto each other? Also make explicit\nor (inclusively) allow the frames to be hidden or collapsed with a button?\n\n> Again, welcome to the project.\n\n<3\nThank you!\n\n - João Victor Bonfim, please use any pronouns.\n"},{"id":"445809","messageId":"AM0PR04MB60198B9F4E77A80526463F6AA54F9@AM0PR04MB6019.eurprd04.prod.outlook.com","threadId":"57095","inReplyTo":"YdTqQDnHLY7oUTja@google.com","subject":"Re: Review process improvements","fromName":"Matthias Aßhauer","fromEmail":"mha1993@live.de","sentAt":"2022-01-09T08:26:00Z","receivedAt":"2022-01-09T08:26:09Z","isPatch":false,"sender":{"key":"mha1993@live.de","avatar":"https://avatars.githubusercontent.com/u/6178234?v=4"},"body":"\n\nOn Tue, 4 Jan 2022, Emily Shaffer wrote:\n\n> On Thu, Dec 16, 2021 at 02:46:21PM -0800, Emily Shaffer wrote:\n>\n> Reshuffling context....\n>\n>> As it's reaching the end of the year and the holiday season, I'm\n>> planning to spend the rest of the year with family and away from my work\n>> email as much as possible. Some other Googler Gitters will be working on\n>> and off and may chime in. But I am expecting to be able to engage more\n>> with this mail in the first week of January.\n>\n> Having had a very restful couple of weeks off, I've now read through all\n> the replies to this mail - thank you! - and will summarize as much as I\n> can.\n>\n>> 1. Draft a MAINTAINERS file\n>> One recurring theme from the conversations I had with the Google Git\n>> team was that selecting a topic to review in the first place can\n>> actually be pretty time-consuming. Choosing from the series subject line\n>> or the \"What's cooking\" mail isn't always straightforward - it can be\n>> hard to tell whether the series is working on a goal you are interested\n>> in or in a part of the codebase you care about. It's not feasible for\n>> someone wanting to get involved to review every single series that comes\n>> across the list - but it might be feasible for someone to review every\n>> series in a particular subsystem or topic.\n>>\n>> In the first half of January, I'd like to have a review out to the list\n>> adding a kernel-style MAINTAINERS file with a few areas of interest. To\n>> that file, I'd like to add \"R:\" (\"CC me!\") lines to subsystem mailing\n>> lists and interested individuals. My hope is that that will make it easy\n>> for someone to either add themselves as an \"R:\" or to subscribe to the\n>> subsystem list, and then be able to filter their mail based on \"stuff\n>> I'm CC'd to\" or \"stuff sent to git-partial-clone@linux.dev\" - and then\n>> be able to review every patch in that list. I'd like to create lists on\n>> topics where it makes sense, too.\n>\n> Summary: Everybody pretty much hated this idea. ;) I count at least five\n> replies saying \"please let's not, it's too cumbersome\".\n>\n> That makes sense to me. As Ævar mentioned - just because he and I are\n> working on config a lot right now, doesn't mean we will still care in\n> 2023. And remembering to remove yourself from the MAINTAINERS file is a\n> pain.\n>\n> Ævar suggested leaning in harder to improving the tooling around \"who do\n> I CC?\"\n> (https://lore.kernel.org/git/211220.86fsqnwkzs.gmgdl%40evledraar.gmail.com)\n> and I think that's a reasonable approach, if we pair it with updating\n> SubmittingPatches and MyFirstContribution to include recommending\n> running the \"who do I CC?\" script.\n>\n>> Since the Git codebase isn't modularized into subsystems as plainly as\n>> the kernel is, I don't think that the MAINTAINERS list needs to be\n>> machine-parseable yet, although it would be a nice goal. Maybe over time\n>> we will decide we'd rather organize the codebase differently and\n>> implement a machine-parseable MAINTAINERS list to incorporate into a\n>> sendemail-validate hook, or something. So I envision lines looking\n>> something like this (just examples, sorry for directly calling people\n>> out ;) ):\n>>\n>>   Submodules\n>>   submodule.[ch], git-submodule.sh, builtin/submodule.c,\n>>     Documentation/git-submodule.txt, anything else adding a feature involving\n>>     submodules\n>>   R: git-submodules@example.com\n>>   R: emilyshaffer@google.com\n>>\n>>   Config improvements\n>>   Documentation/config/*, config.[ch], builtin/config.c, changes which add new\n>>     config options\n>>   R: emilyshaffer@google.com\n>>   R: avarab@gmail.com\n>>\n>> It would be extremely useful to hear other suggestions from the mailing\n>> list about subsystems which deserve a MAINTAINERS line and possibly a\n>> subsystem mailing list.\n>\n> Randall suggested using a prefix/tagging approach to make it easy for\n> reviewers to filter by topic. I think that's a pretty good idea! But I\n> also think for it to be useful as a filtering tool we'd want to\n> formalize \"this topic goes to this tag\" - which brings us back along to\n> the cumbersome-ness of the MAINTAINERS file.\n>\n> I see a couple options:\n>\n> 1) Eat the maintenance overhead of having a checked-in file mapping\n> filespec/topic to subject line tag, like the sample Randall gave\n> (https://lore.kernel.org/git/00b901d7f2d3%24c0d2ca20%2442785e60%24%40nexbridge.com)\n> but minus the \"Reviewers:\" section:\n>\n> Submodules:\n>  Prefix: SUBM\n>  Files:\n>    submodule.[ch], git-submodule.sh, builtin/submodule.c,\n>    Documentation/git-submodule.txt\n> Platform:\n>  Linux:\n>    Prefix: LINUX\n>  NonStop:\n>    Prefix: NS\n>\n> Having these mails all still delivered to git@vger.kernel.org means that\n> it's not such a big deal if the prefix becomes obsolete, and not\n> including an individual CC list means that nobody needs to remove\n> themselves from CC list after they stop caring about some part of the\n> codebase.\n>\n> An alternative to subject lines like \"Subject: SUBM: share config\n> between blah blah\" might be possible. Some providers (at least Gmail,\n> and based on João's reply-to address, protonmail as well) allow mails\n> addressed to e.g. \"my-name+from-git-list@example.com\" to be delivered to\n> \"my-name@example.com\" regardless of what follows the +; if vger allows\n> this kind of thing, we could save some visual space by adopting the\n> convention that e.g. submodule topics should go to\n> \"git-subm@vger.kernel.org\" and allow folks to write their mail filters\n> accordingly. (I took a poke through IETF RFC5322 to see if this +\n> comment/tag/whatever-it-is is standardized and didn't see anything\n> promising, so maybe this alternative is a bad idea. Or maybe I didn't\n> Google well enough.)\n>\n\nThe IETF calls it subaddressing. It's not strictly speaking standardised.\nThere is one RFC about how to handle subaddressing, RFC5233 [1] and one \nold draft to standardise it [2], but no real standard for it.\n\nThere's a lot more mail providers and MTAs that support it in some way,\nbut some use different separators and RFC5233 even mentions puting the \ndetail part before the user part e.g. \"from-git-list+my-name@example.com\".\n\nOther providers include Yahoo, iCloud, Outlook.com and Pobox [3] and MTAs \ninclude sendmail [4], postfix [3] and MS Exchange [5] (though apparently \ncurrently only Exchange Online and 365 instances), from a cursory search.\n\n[1] https://datatracker.ietf.org/doc/html/rfc5233\n[2] https://tools.ietf.org/id/draft-newman-email-subaddr-01.html\n[3] https://en.wikipedia.org/wiki/Email_address#subaddressing\n[4] https://people.cs.rutgers.edu/~watrous/plus-signs-in-email-addresses.html\n[5] https://docs.microsoft.com/en-us/exchange/recipients-in-exchange-online/plus-addressing-in-exchange-online\n\n> 2) Put that kind of mapping into an easier-to-modify place, like a wiki. The\n> downside is that, as far as I'm aware, we don't have or maintain an\n> existing git.git wiki, so it'd probably end up going unused as people\n> don't want to start looking at yet another discussion area.\n\nFrom my understanding we have a wiki on kernel.org [6] that's pretty \nofficial, but isn't very active.\n\nBest regards\n\nMatthias\n\n[6] https://git.wiki.kernel.org/index.php/Main_Page\n\n>\n> Either way, \"Check this file/page and determine what additional tag to\n> add to your outgoing patches\" would need to be called out in\n> documentation like SubmittingPatches//MyFirstContribution - and modeled\n> by veteran prolific contributors in their own patches.\n>\n> If we're generally happy with the idea of tagging rather than additional\n> lists, then I think there's nothing in the way of beginning to draft\n> changes to either of those docs in order to agree on the standard we\n> want to adopt. But I'd like to make sure that we are in agreement first.\n> Maybe the best way to find that agreement is via the doc patches\n> themselves?\n>\n>> 2. Draft a ReviewingPatches doc\n>> Another theme we discussed was the general ambiguity around the act of\n>> performing code review. How detailed should the review be? Who should be\n>> examining interoperability with the rest of the codebase? Is it OK to\n>> reply with only \"I don't know what you're trying to do, rewrite your\n>> commit message please\"?\n>>\n>> Sometime in January I'd like to have a review out with an outline with\n>> agreement on the content and organization for a ReviewingPatches doc.\n>> I'm hoping that it can give some structure to review by including:\n>>   - How to give different types of review (maintainer vs. individual vs.\n>>     drive-by nits)\n>>   - Review best practices (Sage Sharp's article on stages of review[4],\n>>     CCing experts/lists from MAINTAINERS file above, short-circuiting by\n>>     sending comments early instead of reviewing a patch in-depth when\n>>     the idea seems so-so or unclear)\n>>   - Checklists for new contributors to use or to help make in-depth\n>>     review more thorough\n>>\n>> Since this is intended to be checked into Git's codebase, I expect that\n>> we'll want to discuss a lot and make sure we all buy into the contents\n>> of that doc.\n>\n> This idea seemed generally well-received. I noticed that João was\n> curious to hear more about Google's review process; I'll state that\n> there is less magic than you might think, and whatever we do know will\n> certainly make it into a draft.\n>\n> I think there is nothing standing in the way of sending out an outline\n> and/or first draft of such a document for us to all discuss and quibble\n> over. :)\n>\n>>\n>> 3. Google Git team will review cover letters and commit messages\n>>    internally before sending series to the list\n>> I often find that when I send a patch to the list, reviewers reply\n>> without understanding the point of what I was trying to achieve in the\n>> patch. It sounded like this happens to some other Google Gitters as\n>> well. Luckily, that's fixable by the patch author.\n>>\n>> To try and make sure we're sending patches that are easy to understand\n>> and decide whether to review, we're adding a step to our process ASAP to\n>> require commit messages, cover letters, and \"---\" notes to be reviewed\n>> and approved by at least one other Googler before a topic goes to the\n>> mailing list. Those reviews don't need to be formal, but do need to\n>> happen for every reroll of a series. We also will intentionally *not*\n>> review the diff of changes in this private setting - reviews for code\n>> correctness, etc. should continue to happen upstream, in public.\n>>\n>> I'm mentioning that takeaway in this email to say: if you find you don't\n>> understand the purpose of a patch from a Googler, please let us know! It\n>> would be really valuable for us to receive a review saying \"I think you\n>> might be saying X but it's still confusing because you wrote Y\"\n>> so that we can incorporate the source of your confusion as we continue\n>> to review each other's \"accompanying context\".\n>\n> It seems that folks liked this idea, and furthermore, that it would be\n> worth suggesting such a process - \"ask your friend to read your commit\n> message!\" - in one or both of SubmittingPatches and MyFirstContribution.\n>\n>>\n>> 4. Documentation changes to encourage commit message quality\n>\n> I think I didn't hear much response to this other than \"sure, sounds\n> fine\" - so I expect that with the changes themselves will come more\n> detailed discussion. I also don't think there is anything stopping these\n> changes from happening.\n>\n> ---\n>\n> In general, I think everybody seems enough on board for us to begin\n> sending doc patches. So the next step for me is to work out which of we\n> Googlers will handle which, and for us to start sending stuff for\n> review. Thank you to everybody who contributed on this thread and thanks\n> in advance for reviews on the patches to come.\n>\n> - Emily\n>\n"},{"id":"445827","messageId":"nycvar.QRO.7.76.6.2201101401530.339@tvgsbejvaqbjf.bet","threadId":"57095","inReplyTo":"xmqqh7b0juk5.fsf@gitster.g","subject":"Why GitGitGadget does not use Sender:, was Re: Review process improvements","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-01-10T13:03:01Z","receivedAt":"2022-01-10T13:04:55Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 22 Dec 2021, Junio C Hamano wrote:\n\n> Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:\n>\n> > This would require pretending that we're authorized to send mail from the\n> > domain name of the commit author, so this unfortunately won't work (and hence\n> > the reason why GGG does it this way). E.g. say you have:\n> >\n> > From: foo@redhat.com\n> > Subject: [PATCH] Fix foo\n>\n> Would it help to use \"Sender:\"?\n\nYou had suggested that before, and I explained why it does not work in\nhttps://lore.kernel.org/git/nycvar.QRO.7.76.6.2008241445200.56@tvgsbejvaqbjf.bet/.\nThe gist is: GMail interferes with your cunning plan.\n\n> When GGG or any other automation are trying to send e-mail on behalf of\n> the person shown on \"From:\", I thought that it is the mechanism for them\n> to use to identify themselves.\n\nCiao,\nDscho\n"},{"id":"445840","messageId":"xmqqwnj71pl4.fsf@gitster.g","threadId":"57095","inReplyTo":"nycvar.QRO.7.76.6.2201101401530.339@tvgsbejvaqbjf.bet","subject":"Re: Why GitGitGadget does not use Sender:, was Re: Review process improvements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-10T17:13:27Z","receivedAt":"2022-01-10T17:13:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Konstantin gave a more generally applicable answer to think about\nthis issue in <20211222213247.5dnj3zlj53lh6l32@meerkat.local> in a\nseparate thread last year.  I would not be surprised if GMail pays\nattention to the reasoning he gave in that message and that is the\nreason why it does not use the \"Sender\" as I hoped.  I expect other\ne-mail providers should do the same.\n\nThanks.  \n"}]}