{"thread":{"id":"47953","subject":"[RFC] Contributing to Git (on Windows)","startedAt":"2018-03-01T17:35:47Z","lastAt":"2018-03-07T14:16:41Z","messageCount":14,"participants":["Derrick Stolee","Jonathan Nieder","Junio C Hamano","Johannes Schindelin","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"340737","messageId":"176408fc-3645-66d3-2aa3-30ca3fa723e7@gmail.com","threadId":"47953","inReplyTo":null,"subject":"[RFC] Contributing to Git (on Windows)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-03-01T17:35:39Z","receivedAt":"2018-03-01T17:35:47Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"We (Git devs at Microsoft) have had several people start contributing to \nGit over the past few years (I'm the most-recent addition). As we \non-boarded to Git development on our Windows machines, we collected our \nsetup steps on an internal wiki page.\n\nNow, we'd like to make that document publicly available. These steps are \nfocused on a Windows user, so we propose putting them in the \ngit-for-windows/git repo under CONTRIBUTING.md. I have a pull request \nopen for feedback [1]. I'll read comments on that PR or in this thread.\n\nIf you've ever done Git development on a Windows machine, I would love \nto hear your feedback on this document. Any other advice you have is \ngreatly appreciated.\n\nFor anyone interested, there are also a discussion about submitting \npatches upstream. The document links to Documentation/CodingGuidelines \nand Documentation/SubmittingPatches, but tries to elaborate with \nadditional advice.\n\nThanks,\n-Stolee\n\n[1] https://github.com/git-for-windows/git/pull/1529\n"},{"id":"340782","messageId":"20180302044409.GC238112@aiede.svl.corp.google.com","threadId":"47953","inReplyTo":"176408fc-3645-66d3-2aa3-30ca3fa723e7@gmail.com","subject":"Re: [RFC] Contributing to Git (on Windows)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-03-02T04:44:09Z","receivedAt":"2018-03-02T04:44:20Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nDerrick Stolee wrote:\n\n> Now, we'd like to make that document publicly available. These steps are\n> focused on a Windows user, so we propose putting them in the\n> git-for-windows/git repo under CONTRIBUTING.md. I have a pull request open\n> for feedback [1]. I'll read comments on that PR or in this thread.\n\nThanks!  I wonder if we can put this in the standard Documentation/\ndirectory as well.  E.g. the Windows CONTRIBUTING.md could say say\n\"See Documentation/Contributing-On-Windows.md\" so that the bulk of the\ntext would not have to be git-for-windows specific.\n\n[...]\n> +++ b/CONTRIBUTING.md\n> @@ -0,0 +1,401 @@\n> +How to Contribute to Git for Windows\n> +====================================\n\nWould it make sense for this to be checked in with LF instead of CRLF\nline endings, so that clones use the user's chosen / platform native\nline ending?  The .gitattributes file could include\n\n\t/CONTRIBUTING.md text\n\nso that line ending conversion takes place even if the user hasn't\nenabled core.autocrlf.\n\n[...]\n> +    The SDK uses a different credential manager, so you may still want to use Visual Studio\n> +    or normal Git Bash for interacting with your remotes.  Alternatively, use SSH rather\n> +    than HTTPS and avoid credential manager problems.\n\nWhere can I read more about the problems in question?\n\n[...]\n> +Many new contributors ask: What should I start working on?\n> +\n> +One way to win big with the maintainer of an open-source project is to look at the\n> +[issues page](https://github.com/git-for-windows/git/issues) and see if there are any issues that\n> +you can fix quickly, or if anything catches your eye.\n\n<shameless plug>You can also look at https://crbug.com/git for non\nWindows-specific issues.  And you can look at recent user questions\non the mailing list: https://public-inbox.org/git\n\n[...]\n> +If you are adding new functionality, you may need to create low-level tests by creating\n> +helper commands that test a very limited action. These commands are stored in `t/helpers`.\n> +When adding a helper, be sure to add a line to `t/Makefile` and to the `.gitignore` for the\n> +binary file you add. The Git community prefers functional tests using the full `git`\n> +executable, so be sure the test helper is required.\n\nMaybe s/low-level/unit/?\n\n[...]\n> +Read [`t/README`](https://github.com/git/git/blob/master/t/README) for more details.\n\nForgive my ignorance: does github flavored markdown allow relative\nlinks?  (I.e., could this say [`t/README`](t/README)?)\n\n[...]\n> +You can also set certain environment variables to help test the performance on different\n> +repositories or with more repetitions. The full list is available in\n> +[the `t/perf/README` file](https://github.com/git/git/blob/master/t/perf/README),\n\nLikewise.\n\n[...]\n> +Test Your Changes on Linux\n> +--------------------------\n> +\n> +It can be important to work directly on the [core Git codebase](https://github.com/git/git),\n> +such as a recent commit into the `master` or `next` branch that has not been incorporated\n> +into Git for Windows. Also, it can help to run functional and performance tests on your\n> +code in Linux before submitting patches to the Linux-focused mailing list.\n\nI'm surprised at this advice.  Does it actually come up?  When I was\non Mac I never worried about this, nor when I was on Windows.\n\nI'm personally happy to review patches that haven't been tested on\nLinux, though it's of course even nicer if the patch author mentions\nwhat platforms they've tested on.\n\nMaybe this can be reframed to refer specifically to cases where you've\nmodified some Linux platform-specific code (e.g. to add a new feature\nto run-command.c)?\n\n[...]\n> +When preparing your patch, it is important to put yourself in the shoes of the maintainer.\n\n... and in the shoes of other users and developers working with Git down\nthe line who will interact with the patch later!\n\nActually, the maintainer is one of the least important audiences for a\ncommit message.  But may 'the maintainer' is a stand-in for 'the\nproject' here?\n\n[...]\n> +* Make sure the commits are signed off using `git commit (-s|--signoff)`. This means that you\n> +  testify to be allowed to contribute your changes, in particular that if you did this on company\n> +  time, said company approved to releasing your work as Open Source under the GNU Public License v2.\n\nCan this either point to or quote the relevant legal text from\nDocumentation/SubmittingPatches?  It feels dangerous (in the sense of,\npotentially leading to misunderstandings and major future pain) to ask\npeople to sign off without them knowing exactly what that means.\n\nThe rest also looks nice.  Thanks for working on this.\n\nSincerely,\nJonathan\n"},{"id":"340806","messageId":"c53cc179-8102-76a4-1b04-a1107e75e790@gmail.com","threadId":"47953","inReplyTo":"20180302044409.GC238112@aiede.svl.corp.google.com","subject":"Re: [RFC] Contributing to Git (on Windows)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-03-02T13:38:29Z","receivedAt":"2018-03-02T13:38:39Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/1/2018 11:44 PM, Jonathan Nieder wrote:\n> Hi,\n>\n> Derrick Stolee wrote:\n>\n>> Now, we'd like to make that document publicly available. These steps are\n>> focused on a Windows user, so we propose putting them in the\n>> git-for-windows/git repo under CONTRIBUTING.md. I have a pull request open\n>> for feedback [1]. I'll read comments on that PR or in this thread.\n> Thanks!  I wonder if we can put this in the standard Documentation/\n> directory as well.  E.g. the Windows CONTRIBUTING.md could say say\n> \"See Documentation/Contributing-On-Windows.md\" so that the bulk of the\n> text would not have to be git-for-windows specific.\n\nThat's a good idea. After this review stabilizes, I'll send a patch to \nadd the windows-specific instructions as you recommend.\n\nWhat precedent do we have for referencing forks like git-for-windows/git?\n\n>\n> [...]\n>> +++ b/CONTRIBUTING.md\n>> @@ -0,0 +1,401 @@\n>> +How to Contribute to Git for Windows\n>> +====================================\n> Would it make sense for this to be checked in with LF instead of CRLF\n> line endings, so that clones use the user's chosen / platform native\n> line ending?  The .gitattributes file could include\n>\n> \t/CONTRIBUTING.md text\n>\n> so that line ending conversion takes place even if the user hasn't\n> enabled core.autocrlf.\n\nOops! I turned off the CRLF munging in my repo because I had an issue \nwith a script somewhere, but forgot to turn it back on again. Thanks for \nchecking this.\n\n>\n> [...]\n>> +    The SDK uses a different credential manager, so you may still want to use Visual Studio\n>> +    or normal Git Bash for interacting with your remotes.  Alternatively, use SSH rather\n>> +    than HTTPS and avoid credential manager problems.\n> Where can I read more about the problems in question?\n\nI'll try to see if we can elaborate here. The Git for Windows client \nships with Git Credential Manager for Windows [1] but the SDK does not. \nAt the very least, credential interactions are not the same and you do \nnot have access to the credentials stored in Windows Credential Manager.\n\n[1] https://github.com/Microsoft/Git-Credential-Manager-for-Windows\n\n>\n> [...]\n>> +Many new contributors ask: What should I start working on?\n>> +\n>> +One way to win big with the maintainer of an open-source project is to look at the\n>> +[issues page](https://github.com/git-for-windows/git/issues) and see if there are any issues that\n>> +you can fix quickly, or if anything catches your eye.\n> <shameless plug>You can also look at https://crbug.com/git for non\n> Windows-specific issues.  And you can look at recent user questions\n> on the mailing list: https://public-inbox.org/git\n>\n> [...]\n>> +If you are adding new functionality, you may need to create low-level tests by creating\n>> +helper commands that test a very limited action. These commands are stored in `t/helpers`.\n>> +When adding a helper, be sure to add a line to `t/Makefile` and to the `.gitignore` for the\n>> +binary file you add. The Git community prefers functional tests using the full `git`\n>> +executable, so be sure the test helper is required.\n> Maybe s/low-level/unit/?\n>\n> [...]\n>> +Read [`t/README`](https://github.com/git/git/blob/master/t/README) for more details.\n> Forgive my ignorance: does github flavored markdown allow relative\n> links?  (I.e., could this say [`t/README`](t/README)?)\n>\n> [...]\n>> +You can also set certain environment variables to help test the performance on different\n>> +repositories or with more repetitions. The full list is available in\n>> +[the `t/perf/README` file](https://github.com/git/git/blob/master/t/perf/README),\n> Likewise.\n>\n> [...]\n>> +Test Your Changes on Linux\n>> +--------------------------\n>> +\n>> +It can be important to work directly on the [core Git codebase](https://github.com/git/git),\n>> +such as a recent commit into the `master` or `next` branch that has not been incorporated\n>> +into Git for Windows. Also, it can help to run functional and performance tests on your\n>> +code in Linux before submitting patches to the Linux-focused mailing list.\n> I'm surprised at this advice.  Does it actually come up?  When I was\n> on Mac I never worried about this, nor when I was on Windows.\n>\n> I'm personally happy to review patches that haven't been tested on\n> Linux, though it's of course even nicer if the patch author mentions\n> what platforms they've tested on.\n>\n> Maybe this can be reframed to refer specifically to cases where you've\n> modified some Linux platform-specific code (e.g. to add a new feature\n> to run-command.c)?\n\nI recently had a bug in my local copy of the commit-graph patch that was \nonly caught by our Mac OSX automated builds: I was passing the \nedge-value for a parent into a lookup to get an octopus edge from the \ngraph, but I forgot to drop the most-significant bit. This cast the \n\"uint32_t\" silently into an \"int\" but since we multiplied by 4 somehow \nthe Windows and Linux compilers turned this into a left-shift while Mac \ndid not and failed during my test.\n\nNow this is an example of something that probably would have been caught \nin review, but stuff gets through.\n\nThe Windows/Linux boundary is usually enough to catch the platform \ndifferences, but they do catch things that you wouldn't think are \nplatform-specific.\n\n>\n> [...]\n>> +When preparing your patch, it is important to put yourself in the shoes of the maintainer.\n> ... and in the shoes of other users and developers working with Git down\n> the line who will interact with the patch later!\n>\n> Actually, the maintainer is one of the least important audiences for a\n> commit message.  But may 'the maintainer' is a stand-in for 'the\n> project' here?\n\nStand-in for \"the community\" yeah.\n\n>\n> [...]\n>> +* Make sure the commits are signed off using `git commit (-s|--signoff)`. This means that you\n>> +  testify to be allowed to contribute your changes, in particular that if you did this on company\n>> +  time, said company approved to releasing your work as Open Source under the GNU Public License v2.\n> Can this either point to or quote the relevant legal text from\n> Documentation/SubmittingPatches?  It feels dangerous (in the sense of,\n> potentially leading to misunderstandings and major future pain) to ask\n> people to sign off without them knowing exactly what that means.\n\n+1 thanks.\n\n>\n> The rest also looks nice.  Thanks for working on this.\n>\n> Sincerely,\n> Jonathan\n\nThanks for the feedback.\n\n-Stolee\n\n"},{"id":"340815","messageId":"xmqq606ee89v.fsf@gitster-ct.c.googlers.com","threadId":"47953","inReplyTo":"20180302044409.GC238112@aiede.svl.corp.google.com","subject":"Re: [RFC] Contributing to Git (on Windows)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-02T16:45:16Z","receivedAt":"2018-03-02T16:45:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> +When adding a helper, be sure to add a line to `t/Makefile` and to the `.gitignore` for the\n>> +binary file you add. The Git community prefers functional tests using the full `git`\n>> +executable, so be sure the test helper is required.\n>\n> Maybe s/low-level/unit/?\n\nYup.  \n\nAlso \"be sure the test helper is required\" at the end did not click\nuntil I read it twice and realized it wanted to say that addition of\nnew test helper executable for function level unit testing is\ndiscouraged and should be limited to cases where it is absolutely\nnecessary.\n\n>> +Test Your Changes on Linux\n>> +--------------------------\n>> +\n>> +It can be important to work directly on the [core Git codebase](https://github.com/git/git),\n>> +such as a recent commit into the `master` or `next` branch that has not been incorporated\n>> +into Git for Windows. Also, it can help to run functional and performance tests on your\n>> +code in Linux before submitting patches to the Linux-focused mailing list.\n>\n> I'm surprised at this advice.  Does it actually come up?  When I was\n> on Mac I never worried about this, nor when I was on Windows.\n>\n> I'm personally happy to review patches that haven't been tested on\n> Linux, though it's of course even nicer if the patch author mentions\n> what platforms they've tested on.\n\ns/Linux/other platforms/, perhaps?  In fact, portability issues in a\npatch originally written for a platform is rather quickly discovered\nif the original platform is more minor than the others, so while it\nis good advice to test your ware before you make it public, singling\nout the portability issues may not add much value.  The fewer number\nof obvious bugs remaining, the fewer number of iterations it would\ntake for a series to get in a good shape.\n\n>> +When preparing your patch, it is important to put yourself in the shoes of the maintainer.\n>\n> ... and in the shoes of other users and developers working with Git down\n> the line who will interact with the patch later!\n>\n> Actually, the maintainer is one of the least important audiences for a\n> commit message.  But may 'the maintainer' is a stand-in for 'the\n> project' here?\n\nI tend to agree.  The reviewers all comment on the proposed log\nmessages and code changes to help making the patch understandable by\nfuture readers (i.e. the most important audicences).  The maintainer\nand senior reviewers may happen to be good at putting themselves in\nthe shoes of future readers to spot potential problems than other\nreviewers can, simply because they have been working on the project\nlonger than others.  But we should stress that the patch should be\nwritten for future readers of the code and history.\n\n> [...]\n>> +* Make sure the commits are signed off using `git commit (-s|--signoff)`. This means that you\n>> +  testify to be allowed to contribute your changes, in particular that if you did this on company\n>> +  time, said company approved to releasing your work as Open Source under the GNU Public License v2.\n>\n> Can this either point to or quote the relevant legal text from\n> Documentation/SubmittingPatches?  It feels dangerous (in the sense of,\n> potentially leading to misunderstandings and major future pain) to ask\n> people to sign off without them knowing exactly what that means.\n\nWhen you can point at an existing test in legal context, it is safer\nto do so rather than attempting to \"rephrase\" it yourself (unless\nyou are a lawyer, of course, in which case you can assess the best\ncourse of action yourself).\n"},{"id":"340958","messageId":"nycvar.QRO.7.76.6.1803031721590.56@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"47953","inReplyTo":"xmqq606ee89v.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC] Contributing to Git (on Windows)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-03T17:34:40Z","receivedAt":"2018-03-03T17:34:52Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 2 Mar 2018, Junio C Hamano wrote:\n\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n> \n> >> +Test Your Changes on Linux\n> >> +--------------------------\n> >> +\n> >> +It can be important to work directly on the [core Git\n> >> codebase](https://github.com/git/git), +such as a recent commit into\n> >> the `master` or `next` branch that has not been incorporated +into\n> >> Git for Windows. Also, it can help to run functional and performance\n> >> tests on your +code in Linux before submitting patches to the\n> >> Linux-focused mailing list.\n> >\n> > I'm surprised at this advice.  Does it actually come up?\n\nYes.\n\nI personally set up the automated builds on Windows, Linux and macOS for\nour team, and as a rule we always open an internal Pull Request on our\ntopic branches as we develop them, and you would probably not believe the\nnumber of issues we caught before sending the patches to this list. Issues\nincluding\n\n- scripts not being marked executable (on Windows, we don't care, but you\n  Linux folks do),\n\n- use of symbols that are only available on Windows, but that were not\n  guarded by appropriate `#ifdef`s,\n\n- printf formats available only on Windows,\n\n- EOL_NATIVE differences,\n\n- differences between BSD and GNU tools, such as the white-space in the\n  output of `wc`, the syntactic differences of `sed`'s `-i` option,\n  different options of the `stat` utility to do the same thing, etc\n  (granted, this would not be caught when only testing on Windows and\n  Linux, as both development environments would only use the GNU variety)\n\nOf course, these builds also catch problems by setting DEVELOPER=1 which\ndevelopers might have forgotten to specify when building locally (it\nreally is too easy to forget).\n\n> > When I was on Mac I never worried about this, nor when I was on\n> > Windows.\n\nI commend your boldness.\n\nOf course, you are an old-timer. These instructions are intended for\nnew-timers, even first-timers, who may very well be very scared of reviews\ncriticizing them for, say, not marking a new test script as executable.\n\nIf you ever spoke to potential contributors who decided not to contribute\nafter all, and you dig a little deeper, you might get the same impression\nas I got: they are (rightfully) scared of receiving answers with tons of\ncomments what they did wrong.\n\nMost contributors seem to prefer to be \"criticized\" by tools, preferably\neven during the build process, where they can fix things before anybody\nsees their patches.\n\nThis is, for example, the reason why the cURL project has this really nice\n`lib/checksrc.pl` script that they recommend you run to verify that the\ncode you wrote abides by a few of their source code conventions they have.\nDevelopers run it, see what is wrong, fix it, and no human judges them.\nThey submit the code, and the discussions revolve about how to improve the\npatch to cover more cases or some such.\n\nFor that same reason, namely to make it less harsh for new contributors, I\nalso tried to set up an automated job that tries to reformat patches using\n`clang-format` (without forcing any contributor to try to set up a\nbleeding-edge CLang, which can get very involved, very quickly) and pushes\nan updated topic branch if changes were needed. Sadly, this job is broken\nbecause I seem to be unable to get the `apt-get` commands right to get\nCLang's nightly builds (this job runs on Linux).\n\nTo my chagrin, this idea of making most of the boring stuff (and I include\nformatting in that category, but I am probably special in that respect) as\nautomatable, and as little of an issue for review as possible, leaving\nmost brain cycles to work on the correctness of the patches instead, did\nnot really receive a lot of traction on this mailing list.\n\nWe still have no satisfactory automated way to check contributors' patches\nagainst our (quite fuzzy) coding style. Until that happens, contributors\nwill be constantly faced with reviews about overly long lines, about\ngrammar issues, about commit messages missing Signed-off-by: lines, about\ncommit messages that are too short, etc. Those are all comments that\nnot exactly make contributors feel welcome. And that comments also\ndistract from more interesting use of reviewers' time (such as suggesting\nhelper functions or data structures that the contributor could use to\nsimplify the patches).\n\nBut one thing that we *can* recommend to contributors (in particular\nWindows-based ones) is to verify that their patches work also on Linux, so\nthat they can avoid receiving comments about that class of avoidable\nissues.\n\n> > I'm personally happy to review patches that haven't been tested on\n> > Linux, though it's of course even nicer if the patch author mentions\n> > what platforms they've tested on.\n\nJonathan, I don't want this to sound harsh... but... this contributors'\nguide cares a little more about the feelings of the contributors than\nyours? ;-)\n\nSeriously again, you might not mind pointing out that a newly-added test\ncase is not marked executable. The emotional effect of such a comment on\nan eager, first-time Open Source contributors is very different, though.\nIt is something that can turn a first-time contributor into a\none-time-only contributor.\n\n> s/Linux/other platforms/, perhaps?\n\nNo, this advice comes straight from my personal observation that the\nreviewers on the Git mailing list are Linux-centric.\n\nAnd this advice served my colleagues very well, before we had set up the\nautomated Windows/Linux/macOS builds on our internal Pull Requests.\n\nPlus. We need to stay reasonable. This guide is trying to guide first-time\ncontributors how to contribute to the Git project, with a slight bias\ntowards Windows-based contributors. It is very easy for them to get access\nto a Linux VM: they simply use the Hyper-V Manager (or obtain VirtualBox)\nand install a freely-downloadable Ubuntu .iso.\n\nNow, how reasonable do I think it is to ask those contributors to purchase\nan Apple machine to test their patches on macOS (you cannot just download\nan .iso nor would it be legal to run a macOS VM on anything but Apple\nhardware)? You probably guessed my answer: not at all.\n\nAnd it does not make sense to suggest testing e.g. on FreeBSD:\n\n1) there had been known breakages of our test suite on FreeBSD, and\n\n2) historically, Linux, macOS and Windows are the most important Operating\nSystems when it comes to reviews on the Git mailing list, in that order.\n(I would actually disagree that this reflects the number of respective Git\nusers, but that is another story, and it would be very hard to obtain\ntrustworthy evidence either.)\n\n> In fact, portability issues in a patch originally written for a platform\n> is rather quickly discovered if the original platform is more minor than\n> the others, so while it is good advice to test your ware before you make\n> it public, singling out the portability issues may not add much value.\n> The fewer number of obvious bugs remaining, the fewer number of\n> iterations it would take for a series to get in a good shape.\n\nYou put yourself into the shoes of the maintainer, I see, as asked for by\nStolee's document.\n\nFor you, Junio, however, the task *now* is to put yourself into the shoes\nof a Computer Science student in their 2nd year who wants to become an\nOpen Source contributor and is a little afraid to talk directly to \"the\ncore committers\", and quite scared what negative feedback they might\nreceive.\n\n\"What if they say my code is not good enough?\"\n\nThe document we are discussing here is intended to be useful for these\npotential contributors.\n\nTo you, it may very well look like a great thing if they used a \"minor\"\nplatform (such as Windows, you clearly thought ;-)) so your review would\nmake their code portable enough to work even on Linux. To the first-time\ncontributors *I* know, it looks like a very terrifying thing. \"The\nmaintainer hates my patches!\"\n\nSo. I would rather want to be nice to contributors who have not yet\ncontributed code to the Git mailing list.\n\n> >> +When preparing your patch, it is important to put yourself in the\n> >> shoes of the maintainer.\n> >\n> > ... and in the shoes of other users and developers working with Git down\n> > the line who will interact with the patch later!\n> >\n> > Actually, the maintainer is one of the least important audiences for a\n> > commit message.  But may 'the maintainer' is a stand-in for 'the\n> > project' here?\n> \n> I tend to agree.  The reviewers all comment on the proposed log\n> messages and code changes to help making the patch understandable by\n> future readers (i.e. the most important audicences).  The maintainer\n> and senior reviewers may happen to be good at putting themselves in\n> the shoes of future readers to spot potential problems than other\n> reviewers can, simply because they have been working on the project\n> longer than others.  But we should stress that the patch should be\n> written for future readers of the code and history.\n\nIs this not missing the point of this sentence? Those \"senior reviewers\"\nalso put themselves into the shoes of the maintainer, because considering\nfuture readers is kind of the typical responsibility of the maintainer.\nAnd considering how maintainable the code is. And how correct it is. How\neasy it will be to spot regressions.\n\nSo \"put yourself into the shoes of the maintainer\" is a pretty good advice\nhere, because it will automatically also catch all those other groups of\npeople you mentioned, without mentioning them.\n\n> > [...]\n> >> +* Make sure the commits are signed off using `git commit\n> >> (-s|--signoff)`. This means that you testify to be allowed to\n> >> contribute your changes, in particular that if you did this on\n> >> company time, said company approved to releasing your work as Open\n> >> Source under the GNU Public License v2.\n> >\n> > Can this either point to or quote the relevant legal text from\n> > Documentation/SubmittingPatches?  It feels dangerous (in the sense of,\n> > potentially leading to misunderstandings and major future pain) to ask\n> > people to sign off without them knowing exactly what that means.\n> \n> When you can point at an existing test in legal context, it is safer\n> to do so rather than attempting to \"rephrase\" it yourself (unless\n> you are a lawyer, of course, in which case you can assess the best\n> course of action yourself).\n\nPlease note: Documentation/SubmittingPatches is not a legal text. At least\nI have not seen any accredited lawyer commenting on the validity of this\ndocument, or how much sense it makes.\n\nI could imagine that it would not hold up in court, given that there is no\nrequired consent to the \"terms\" of Documentation/SubmittingPatches before\nanybody sends any patches, and every contributor could claim that they\nmeant something different by \"Signed-off-by:\", that they had not even read\nDocumentation/SubmittingPatches. It *would* probably be different if the\ncontribution process required opening a Pull Request on GitHub and the\n.github/PULL_REQUEST_TEMPLATE.md stated explicitly what we mean by the\n\"Signed-off-by:\" line.\n\nEven so, Git for Windows' own wiki\n(https://github.com/git-for-windows/git/wiki/Good-commits) links to a\nparticular version of Documentation/SubmittingPatches so that it can\nspecifically mark the DCO:\n\nhttps://github.com/git/git/blob/v2.8.1/Documentation/SubmittingPatches#L234-L286\n\nI would probably want to update that to v2.16.2's version and link to that\nfrom the guide, but definitely keep the current wording (as a preview of\nthe full Developers' Certificate of Origin).\n\nPlease note also: there is a seemingly authoritative site at\nhttps://developercertificate.org/ but it does not even talk about\n\"Signed-off-by:\", so we cannot really link there instead.\n\nCiao,\nDscho\n"},{"id":"340959","messageId":"20180303182723.GA76934@aiede.svl.corp.google.com","threadId":"47953","inReplyTo":"nycvar.QRO.7.76.6.1803031721590.56@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [RFC] Contributing to Git (on Windows)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-03-03T18:27:23Z","receivedAt":"2018-03-03T18:27:35Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Dscho,\n\nJohannes Schindelin wrote:\n>> Jonathan Nieder <jrnieder@gmail.com> writes:\n>>> Dereck Stolee wrote:\n\n>>>> +Test Your Changes on Linux\n>>>> +--------------------------\n>>>> +\n>>>> +It can be important to work directly on the [core Git\n>>>> codebase](https://github.com/git/git), +such as a recent commit into\n>>>> the `master` or `next` branch that has not been incorporated +into\n>>>> Git for Windows. Also, it can help to run functional and performance\n>>>> tests on your +code in Linux before submitting patches to the\n>>>> Linux-focused mailing list.\n>>>\n>>> I'm surprised at this advice.  Does it actually come up?\n>\n> Yes.\n>\n> I personally set up the automated builds on Windows, Linux and macOS for\n> our team, and as a rule we always open an internal Pull Request on our\n> topic branches as we develop them, and you would probably not believe the\n> number of issues we caught before sending the patches to this list. Issues\n> including\n[nice list snipped]\n\nThanks for explaining.  I still am going to push back on the wording\nhere, and here is why:\n\n 1. Approximately 1/2 of the differences you describe apply to Mac as\n    well as Windows.  The advice certainly does not apply on Mac.\n\n    You might object: Mac readers would not be reading this text!  But\n    that is not how humans work: if project documentation (e.g. the\n    CONTRIBUTING.md on GitHub!) says that the project is Linux-focused\n    and if you don't test on Linux then you might as well not bother,\n    then people are going to believe it.\n\n 2. It is not unusual for Linux users to make portability mistakes that\n    are quickly pointed out on list.  If anything, the advice to test on\n    other platforms should apply to contributors on Linux even more.\n    This happens especially often to new contributors, who sometimes use\n    bashisms, etc that get quickly pointed out.\n\n 3. I do not *want* Git to be a Linux-focused project; I want the code\n    to perform well on all popular platforms and for people not to be\n    afraid to make it so.\n\n    If the docs say that all we care about is Linux, then people are\n    likely to be too scared to do the necessary and valuable work of\n    making it work well on Mac, Windows, etc.  The actual mix of\n    contributors doesn't bear it out anyway: a large number of\n    contributors are already on Mac or Windows.\n\nFortunately this is pretty straightforward to fix in the doc: it could\nsay something like \"to the multi-platform focused mailing list\", for\nexample.\n\n[...]\n> To my chagrin, this idea of making most of the boring stuff (and I include\n> formatting in that category, but I am probably special in that respect) as\n> automatable, and as little of an issue for review as possible, leaving\n> most brain cycles to work on the correctness of the patches instead, did\n> not really receive a lot of traction on this mailing list.\n\nHuh?  I'm confident that this is a pretty widely supported idea within\nthe Git project.\n\nI get the feeling you must have misread something or misinterpreted a\ndifferent response.\n\n[...]\n> No, this advice comes straight from my personal observation that the\n> reviewers on the Git mailing list are Linux-centric.\n\nHopefully the clarifications and suggestions higher in this message\nhelp.  If they don't, then I'm nervous about our ability to understand\neach other.\n\n[...]\n> Now, how reasonable do I think it is to ask those contributors to purchase\n> an Apple machine to test their patches on macOS (you cannot just download\n> an .iso nor would it be legal to run a macOS VM on anything but Apple\n> hardware)? You probably guessed my answer: not at all.\n\nAgreed, this is something that needs to be automated (and not via a\nCONTRIBUTING.md file).  As a stopgap, having a section in the\ncontribution instructions about testing using Windows's Linux\nsubsystem is a valuable thing, and thanks for that; I never meant to\nimply otherwise.\n\n[...]\n> On Fri, 2 Mar 2018, Junio C Hamano wrote:\n\n>> In fact, portability issues in a patch originally written for a platform\n>> is rather quickly discovered if the original platform is more minor than\n>> the others, so while it is good advice to test your ware before you make\n>> it public, singling out the portability issues may not add much value.\n>> The fewer number of obvious bugs remaining, the fewer number of\n>> iterations it would take for a series to get in a good shape.\n[...]\n> For you, Junio, however, the task *now* is to put yourself into the shoes\n> of a Computer Science student in their 2nd year who wants to become an\n> Open Source contributor and is a little afraid to talk directly to \"the\n> core committers\", and quite scared what negative feedback they might\n> receive.\n>\n> \"What if they say my code is not good enough?\"\n\nSure, though there is something implied in what is Junio is saying\nthat is useful for such people.\n\nIt is patience.  It is the message that if you miss a portability bug,\nwe won't be disappointed in you, and it in fact happens all the time\nto the best of contributors.\n\nIf there's a straightforward way to convey that in the text, I agree\nwith Junio that it's worth conveying.\n\n[...]\n>>>> +When preparing your patch, it is important to put yourself in the\n>>>> shoes of the maintainer.\n>>>\n>>> ... and in the shoes of other users and developers working with Git down\n>>> the line who will interact with the patch later!\n>>>\n>>> Actually, the maintainer is one of the least important audiences for a\n>>> commit message.  But may 'the maintainer' is a stand-in for 'the\n>>> project' here?\n[...]\n> Is this not missing the point of this sentence? Those \"senior reviewers\"\n> also put themselves into the shoes of the maintainer, because considering\n> future readers is kind of the typical responsibility of the maintainer.\n\nOnly in the sense that what the project does is to \"maintain\".\n(Aside: is developing, deploying, supporting users part of\nmaintenance?  I don't actually think so!  We can call it that if you\nlike, though.).\n\nTo put this discussion on a more practical footing: I see new\ncontributors confusing the maintainer for the audience for their\nchanges all the time, and it hurts them.  They get some useful (and\nsome unuseful, sadly) replies from the project and none from the\nmaintainer and they end up confused: does this mean that the person\nthat matters doesn't even care about my patch?\n\nClarifying that the audience is *not* the maintainer can help.\n\nI would not be surprised if the Git for Windows project works\ndifferently: maybe you have the time, inclination, and ability to\nrespond to every contributor directly early in the process.  There's\nnothing wrong with that, and it probably helps to make the process\neasier for contributors with this particular confusion.  Having a\nsingle person that is ready to talk to them and can stand in for what\nthe entire project needs can make their life easier.\n\nBut that's not sustainable in the Git project.  Larger projects either\nneed multiple maintainers or a decrease in scope of the maintainer's\nrole.  Junio uses the latter, which seems fine to me.\n\nEven something as simple as s/maintainer/maintainers/ would make this\ntext less misleading for people contributing to upstream Git.\n\n[...]\n>>>> +* Make sure the commits are signed off using `git commit\n>>>> (-s|--signoff)`. This means that you testify to be allowed to\n>>>> contribute your changes, in particular that if you did this on\n>>>> company time, said company approved to releasing your work as Open\n>>>> Source under the GNU Public License v2.\n>>>\n>>> Can this either point to or quote the relevant legal text from\n>>> Documentation/SubmittingPatches?  It feels dangerous (in the sense of,\n>>> potentially leading to misunderstandings and major future pain) to ask\n>>> people to sign off without them knowing exactly what that means.\n>>\n>> When you can point at an existing test in legal context, it is safer\n>> to do so rather than attempting to \"rephrase\" it yourself (unless\n>> you are a lawyer, of course, in which case you can assess the best\n>> course of action yourself).\n>\n> Please note: Documentation/SubmittingPatches is not a legal text. At least\n> I have not seen any accredited lawyer commenting on the validity of this\n> document, or how much sense it makes.\n\nThis seems to reflect a misunderstanding of how law works, at least in\nthe United States.\n\nI don't know how one would define what a legal text means.  I assume\nyou mean \"a contract\", and I certainly don't care to evaluate whether\nsome particular text is a contract: I am not a lawyer.\n\nBut there are all kinds of other actions or words with legal\nimplications:\n\n - promises\n - injuries\n - etc, etc, etc\n\nBasically anything someone ever does has potential legal implications.\n\nThe developer's certificate of origin has been reviewed by plenty of\nlawyers.  You can have your own lawyers review it too if you like.\n\nThat's all side-stepping what I was saying, which is that it's bad for\nthe project for people to be signing off without knowing what it\nmeans.\n\n[...]\n> Even so, Git for Windows' own wiki\n> (https://github.com/git-for-windows/git/wiki/Good-commits) links to a\n> particular version of Documentation/SubmittingPatches so that it can\n> specifically mark the DCO:\n>\n> https://github.com/git/git/blob/v2.8.1/Documentation/SubmittingPatches#L234-L286\n\nGreat.  That's the sort of thing I'm asking for in this new\ncontributors document.  Why in the world is it useful to push back\nagainst such a request?\n\nConfused,\nJonathan\n"},{"id":"341024","messageId":"663f3fef-30bc-8900-6070-80ac96cccff0@gmail.com","threadId":"47953","inReplyTo":"20180303182723.GA76934@aiede.svl.corp.google.com","subject":"Re: [RFC] Contributing to Git (on Windows)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-03-05T14:50:52Z","receivedAt":"2018-03-05T14:51:03Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"I really appreciate the feedback on this document, Jonathan.\n\nOn 3/3/2018 1:27 PM, Jonathan Nieder wrote:\n> Hi Dscho,\n>\n> Johannes Schindelin wrote:\n>>> Jonathan Nieder <jrnieder@gmail.com> writes:\n>>>> Dereck Stolee wrote:\n\nnit: s/Dereck/Derrick/ Is my outgoing email name misspelled, or do you \nhave a misspelled contact info for me?\n\n>>>>> +Test Your Changes on Linux\n>>>>> +--------------------------\n>>>>> +\n>>>>> +It can be important to work directly on the [core Git\n>>>>> codebase](https://github.com/git/git), +such as a recent commit into\n>>>>> the `master` or `next` branch that has not been incorporated +into\n>>>>> Git for Windows. Also, it can help to run functional and performance\n>>>>> tests on your +code in Linux before submitting patches to the\n>>>>> Linux-focused mailing list.\n>>>> I'm surprised at this advice.  Does it actually come up?\n>> Yes.\n>>\n>> I personally set up the automated builds on Windows, Linux and macOS for\n>> our team, and as a rule we always open an internal Pull Request on our\n>> topic branches as we develop them, and you would probably not believe the\n>> number of issues we caught before sending the patches to this list. Issues\n>> including\n> [nice list snipped]\n>\n> Thanks for explaining.  I still am going to push back on the wording\n> here, and here is why:\n>\n>   1. Approximately 1/2 of the differences you describe apply to Mac as\n>      well as Windows.  The advice certainly does not apply on Mac.\n>\n>      You might object: Mac readers would not be reading this text!  But\n>      that is not how humans work: if project documentation (e.g. the\n>      CONTRIBUTING.md on GitHub!) says that the project is Linux-focused\n>      and if you don't test on Linux then you might as well not bother,\n>      then people are going to believe it.\n>\n>   2. It is not unusual for Linux users to make portability mistakes that\n>      are quickly pointed out on list.  If anything, the advice to test on\n>      other platforms should apply to contributors on Linux even more.\n>      This happens especially often to new contributors, who sometimes use\n>      bashisms, etc that get quickly pointed out.\n>\n>   3. I do not *want* Git to be a Linux-focused project; I want the code\n>      to perform well on all popular platforms and for people not to be\n>      afraid to make it so.\n>\n>      If the docs say that all we care about is Linux, then people are\n>      likely to be too scared to do the necessary and valuable work of\n>      making it work well on Mac, Windows, etc.  The actual mix of\n>      contributors doesn't bear it out anyway: a large number of\n>      contributors are already on Mac or Windows.\n>\n> Fortunately this is pretty straightforward to fix in the doc: it could\n> say something like \"to the multi-platform focused mailing list\", for\n> example.\n\nI like the \"multi-platform\" wording, and I'll try to use it as often as \npossible. I'll keep the Linux instructions because it is free to set up \na Linux environment and testing in Windows and Linux will catch most of \nthe cross-platform issues.\n\n>\n> [...]\n>> To my chagrin, this idea of making most of the boring stuff (and I include\n>> formatting in that category, but I am probably special in that respect) as\n>> automatable, and as little of an issue for review as possible, leaving\n>> most brain cycles to work on the correctness of the patches instead, did\n>> not really receive a lot of traction on this mailing list.\n> Huh?  I'm confident that this is a pretty widely supported idea within\n> the Git project.\n>\n> I get the feeling you must have misread something or misinterpreted a\n> different response.\n>\n> [...]\n>> No, this advice comes straight from my personal observation that the\n>> reviewers on the Git mailing list are Linux-centric.\n> Hopefully the clarifications and suggestions higher in this message\n> help.  If they don't, then I'm nervous about our ability to understand\n> each other.\n>\n> [...]\n>> Now, how reasonable do I think it is to ask those contributors to purchase\n>> an Apple machine to test their patches on macOS (you cannot just download\n>> an .iso nor would it be legal to run a macOS VM on anything but Apple\n>> hardware)? You probably guessed my answer: not at all.\n> Agreed, this is something that needs to be automated (and not via a\n> CONTRIBUTING.md file).  As a stopgap, having a section in the\n> contribution instructions about testing using Windows's Linux\n> subsystem is a valuable thing, and thanks for that; I never meant to\n> imply otherwise.\n>\n> [...]\n>> On Fri, 2 Mar 2018, Junio C Hamano wrote:\n>>> In fact, portability issues in a patch originally written for a platform\n>>> is rather quickly discovered if the original platform is more minor than\n>>> the others, so while it is good advice to test your ware before you make\n>>> it public, singling out the portability issues may not add much value.\n>>> The fewer number of obvious bugs remaining, the fewer number of\n>>> iterations it would take for a series to get in a good shape.\n> [...]\n>> For you, Junio, however, the task *now* is to put yourself into the shoes\n>> of a Computer Science student in their 2nd year who wants to become an\n>> Open Source contributor and is a little afraid to talk directly to \"the\n>> core committers\", and quite scared what negative feedback they might\n>> receive.\n>>\n>> \"What if they say my code is not good enough?\"\n> Sure, though there is something implied in what is Junio is saying\n> that is useful for such people.\n>\n> It is patience.  It is the message that if you miss a portability bug,\n> we won't be disappointed in you, and it in fact happens all the time\n> to the best of contributors.\n>\n> If there's a straightforward way to convey that in the text, I agree\n> with Junio that it's worth conveying.\n>\n> [...]\n>>>>> +When preparing your patch, it is important to put yourself in the\n>>>>> shoes of the maintainer.\n>>>> ... and in the shoes of other users and developers working with Git down\n>>>> the line who will interact with the patch later!\n>>>>\n>>>> Actually, the maintainer is one of the least important audiences for a\n>>>> commit message.  But may 'the maintainer' is a stand-in for 'the\n>>>> project' here?\n> [...]\n>> Is this not missing the point of this sentence? Those \"senior reviewers\"\n>> also put themselves into the shoes of the maintainer, because considering\n>> future readers is kind of the typical responsibility of the maintainer.\n> Only in the sense that what the project does is to \"maintain\".\n> (Aside: is developing, deploying, supporting users part of\n> maintenance?  I don't actually think so!  We can call it that if you\n> like, though.).\n>\n> To put this discussion on a more practical footing: I see new\n> contributors confusing the maintainer for the audience for their\n> changes all the time, and it hurts them.  They get some useful (and\n> some unuseful, sadly) replies from the project and none from the\n> maintainer and they end up confused: does this mean that the person\n> that matters doesn't even care about my patch?\n>\n> Clarifying that the audience is *not* the maintainer can help.\n>\n> I would not be surprised if the Git for Windows project works\n> differently: maybe you have the time, inclination, and ability to\n> respond to every contributor directly early in the process.  There's\n> nothing wrong with that, and it probably helps to make the process\n> easier for contributors with this particular confusion.  Having a\n> single person that is ready to talk to them and can stand in for what\n> the entire project needs can make their life easier.\n>\n> But that's not sustainable in the Git project.  Larger projects either\n> need multiple maintainers or a decrease in scope of the maintainer's\n> role.  Junio uses the latter, which seems fine to me.\n>\n> Even something as simple as s/maintainer/maintainers/ would make this\n> text less misleading for people contributing to upstream Git.\n\nI really like the suggestion to swap \"maintainer\" for \"community\". It's \nsomething I've learned by being on the list since I first wrote that \nparagraph about \"the maintainer\" in October. Back then, I was new to the \nlist and my main way of understanding what was going on was by reading \nJunio's \"What's Cooking\" emails.\n\n> [...]\n>>>>> +* Make sure the commits are signed off using `git commit\n>>>>> (-s|--signoff)`. This means that you testify to be allowed to\n>>>>> contribute your changes, in particular that if you did this on\n>>>>> company time, said company approved to releasing your work as Open\n>>>>> Source under the GNU Public License v2.\n>>>> Can this either point to or quote the relevant legal text from\n>>>> Documentation/SubmittingPatches?  It feels dangerous (in the sense of,\n>>>> potentially leading to misunderstandings and major future pain) to ask\n>>>> people to sign off without them knowing exactly what that means.\n>>> When you can point at an existing test in legal context, it is safer\n>>> to do so rather than attempting to \"rephrase\" it yourself (unless\n>>> you are a lawyer, of course, in which case you can assess the best\n>>> course of action yourself).\n>> Please note: Documentation/SubmittingPatches is not a legal text. At least\n>> I have not seen any accredited lawyer commenting on the validity of this\n>> document, or how much sense it makes.\n> This seems to reflect a misunderstanding of how law works, at least in\n> the United States.\n>\n> I don't know how one would define what a legal text means.  I assume\n> you mean \"a contract\", and I certainly don't care to evaluate whether\n> some particular text is a contract: I am not a lawyer.\n>\n> But there are all kinds of other actions or words with legal\n> implications:\n>\n>   - promises\n>   - injuries\n>   - etc, etc, etc\n>\n> Basically anything someone ever does has potential legal implications.\n>\n> The developer's certificate of origin has been reviewed by plenty of\n> lawyers.  You can have your own lawyers review it too if you like.\n>\n> That's all side-stepping what I was saying, which is that it's bad for\n> the project for people to be signing off without knowing what it\n> means.\n>\n> [...]\n>> Even so, Git for Windows' own wiki\n>> (https://github.com/git-for-windows/git/wiki/Good-commits) links to a\n>> particular version of Documentation/SubmittingPatches so that it can\n>> specifically mark the DCO:\n>>\n>> https://github.com/git/git/blob/v2.8.1/Documentation/SubmittingPatches#L234-L286\n> Great.  That's the sort of thing I'm asking for in this new\n> contributors document.  Why in the world is it useful to push back\n> against such a request?\n>\n\nI have a habit of being too loose in language around lawyer-speak. I \nshould not have attempted to summarize what \"Signed-off-by:\" means and \nwill use that helpful link for the description instead.\n\nThanks,\n-Stolee\n\n"},{"id":"341026","messageId":"nycvar.QRO.7.76.6.1803051711210.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"47953","inReplyTo":"20180303182723.GA76934@aiede.svl.corp.google.com","subject":"Re: [RFC] Contributing to Git (on Windows)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-05T16:42:21Z","receivedAt":"2018-03-05T16:42:36Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jonathan,\n\nOn Sat, 3 Mar 2018, Jonathan Nieder wrote:\n\n> Johannes Schindelin wrote:\n> >> Jonathan Nieder <jrnieder@gmail.com> writes:\n> >>> Dereck Stolee wrote:\n> \n> >>>> +Test Your Changes on Linux\n> >>>> +--------------------------\n> >>>> +\n> >>>> +It can be important to work directly on the [core Git\n> >>>> codebase](https://github.com/git/git), +such as a recent commit into\n> >>>> the `master` or `next` branch that has not been incorporated +into\n> >>>> Git for Windows. Also, it can help to run functional and performance\n> >>>> tests on your +code in Linux before submitting patches to the\n> >>>> Linux-focused mailing list.\n> >>>\n> >>> I'm surprised at this advice.  Does it actually come up?\n> >\n> > Yes.\n> >\n> > I personally set up the automated builds on Windows, Linux and macOS for\n> > our team, and as a rule we always open an internal Pull Request on our\n> > topic branches as we develop them, and you would probably not believe the\n> > number of issues we caught before sending the patches to this list. Issues\n> > including\n> [nice list snipped]\n> \n> Thanks for explaining.  I still am going to push back on the wording\n> here, and here is why:\n> \n>  1. Approximately 1/2 of the differences you describe apply to Mac as\n>     well as Windows.\n\nNo. The executable bit, the native line endings, most of the issues I\nlisted do not catch macOS-based developers off guard.\n\n> The advice certainly does not apply on Mac.\n\nThat is true.\n\nStolee agreed in the PR to mention alternatives to Hyper-V, such as\nVirtualBox, which would help macOS-based developers here.\n\n>     You might object: Mac readers would not be reading this text!  But\n>     that is not how humans work: if project documentation (e.g. the\n>     CONTRIBUTING.md on GitHub!) says that the project is Linux-focused\n>     and if you don't test on Linux then you might as well not bother,\n>     then people are going to believe it.\n\nI thought the document encourages to test on Linux. It does not claim that\nyou can forget about getting your patches accepted if you don't test on\nLinux.\n\n>  2. It is not unusual for Linux users to make portability mistakes that\n>     are quickly pointed out on list.  If anything, the advice to test on\n>     other platforms should apply to contributors on Linux even more.\n>     This happens especially often to new contributors, who sometimes use\n>     bashisms, etc that get quickly pointed out.\n\nTrue. Maybe this document is not for Linux-based developers, then. I might\nover-generalize here, but in my experience, Windows-based developers were\nparticularly uneasy about getting what they perceived as criticized, and\nfor those developers it is that I want tools to tell them what is wrong.\nIt makes it much easier to accept.\n\n>  3. I do not *want* Git to be a Linux-focused project; I want the code\n>     to perform well on all popular platforms and for people not to be\n>     afraid to make it so.\n\nMe, too.\n\nRealistically, though, way more than half of the reviewers on the Git\nmailing list use Linux, and rarely switch to any other OS even for\ntesting.\n\nThe Google gang (you & Junio included) uses Linux. Peff uses Linux. From\nwhat I can see Duy, Eric and Jake use Linux. That covers already the most\nactive reviewers right there.\n\nSo in order to get patches accepted (and that is the goal of a\ncontributor), chances can be improved by making stuff work on Linux.\n\n>     If the docs say that all we care about is Linux, then people are\n>     likely to be too scared to do the necessary and valuable work of\n>     making it work well on Mac, Windows, etc.  The actual mix of\n>     contributors doesn't bear it out anyway: a large number of\n>     contributors are already on Mac or Windows.\n\nThis document targets Windows-based developers. So they will already have\nmade sure that it works well on Windows.\n\nAlso, putting the burden of testing portability on first-time contributors\nis really what will scare them away. So don't pretend that you want to\navoid scaring them if you talk about portability.\n\n> Fortunately this is pretty straightforward to fix in the doc: it could\n> say something like \"to the multi-platform focused mailing list\", for\n> example.\n\nYou know what? I'd rather not. Seriously. I am already uneasy with\nsuggesting to install a full-fledged Linux VM, but I think it will\nultimately help contributors *not* to get scared away already by the first\nvolley of reviewer comments.\n\n> [...]\n> > To my chagrin, this idea of making most of the boring stuff (and I include\n> > formatting in that category, but I am probably special in that respect) as\n> > automatable, and as little of an issue for review as possible, leaving\n> > most brain cycles to work on the correctness of the patches instead, did\n> > not really receive a lot of traction on this mailing list.\n> \n> Huh?  I'm confident that this is a pretty widely supported idea within\n> the Git project.\n> \n> I get the feeling you must have misread something or misinterpreted a\n> different response.\n\nSo where is that formatter call that fixes your code? Where is that\nstatement that says that we have a consistent code formatting, and don't\nlet taste override automatic formatting settings?\n\nAs long as you allow taste to rule the coding style where automated tools\ncould prettify code consistently, you will always have those discussions\n(that I personally really dislike, as I find that they distract from my\n*code*).\n\n> [...]\n> > No, this advice comes straight from my personal observation that the\n> > reviewers on the Git mailing list are Linux-centric.\n> \n> Hopefully the clarifications and suggestions higher in this message\n> help.  If they don't, then I'm nervous about our ability to understand\n> each other.\n\nOkay, let me state what I think the goal of this document should be:\n\n\tTo help Windows-based developers who want to contribute their first\n\tpatch to the Git project.\n\nWhatever makes it easier to contributors and is easily explained, should\ngo in, in my opinion.\n\nWishful thinking, and philosophical considerations, should probably stay\nout.\n\nYes, in an ideal world contributors would read this document and then\nunderstand what it takes to contribute to this heterogenous project. But\nthat is impractical. Already you and me disagree on many issues. Those\ndisagreements do not need to be part of that document.\n\n> [...]\n> > Now, how reasonable do I think it is to ask those contributors to purchase\n> > an Apple machine to test their patches on macOS (you cannot just download\n> > an .iso nor would it be legal to run a macOS VM on anything but Apple\n> > hardware)? You probably guessed my answer: not at all.\n> \n> Agreed, this is something that needs to be automated (and not via a\n> CONTRIBUTING.md file).  As a stopgap, having a section in the\n> contribution instructions about testing using Windows's Linux\n> subsystem is a valuable thing, and thanks for that; I never meant to\n> imply otherwise.\n\nFor the moment, we have Travis CI. Since it leaves out Windows, it does\nnot help my job one bit, and whatever I did to get Windows testing into\nour Continuous Testing (it is *not* Continuous Integration, you probably\nagree with me on *that*) is but a stop-gap measure.\n\nI am part of the organization developing Visual Studio Team Services\n(VSTS), the integrated DevOps solution also used to develop Windows using\nthe largest repository on this planet, and as such have the privilege of\nbeing able to use it to my heart's extent.  It has build agents for\nWindows, Linux and macOS. I hope to get a bit more time soon to play with\nthem and get some more robust testing in place by copy/editing the build\ndefinitions I already use for our internal PRs.\n\nThe downside is that the logs are not public.\n\nHowever, VSTS is free for teams up to five, meaning that any developer can\nregister a project, and once I manage to get build definitions in shape, I\ncan make them public and anybody can test their code on the major three\nplatforms, in their personal VSTS account.\n\n> [...]\n> > On Fri, 2 Mar 2018, Junio C Hamano wrote:\n> \n> >> In fact, portability issues in a patch originally written for a platform\n> >> is rather quickly discovered if the original platform is more minor than\n> >> the others, so while it is good advice to test your ware before you make\n> >> it public, singling out the portability issues may not add much value.\n> >> The fewer number of obvious bugs remaining, the fewer number of\n> >> iterations it would take for a series to get in a good shape.\n> [...]\n> > For you, Junio, however, the task *now* is to put yourself into the shoes\n> > of a Computer Science student in their 2nd year who wants to become an\n> > Open Source contributor and is a little afraid to talk directly to \"the\n> > core committers\", and quite scared what negative feedback they might\n> > receive.\n> >\n> > \"What if they say my code is not good enough?\"\n> \n> Sure, though there is something implied in what is Junio is saying\n> that is useful for such people.\n> \n> It is patience.  It is the message that if you miss a portability bug,\n> we won't be disappointed in you, and it in fact happens all the time\n> to the best of contributors.\n> \n> If there's a straightforward way to convey that in the text, I agree\n> with Junio that it's worth conveying.\n\nThat is not how I read Junio's statement. I read it more like \"If your\ncode is flawed, we'll let you know.\"\n\nBut I like your idea of conveying in the document that you can expect\nkind and patient help in improving your patches. To somehow stave off the\nfeeling of getting criticized, and reframing it instead to tell\ncontributors that this is intended as helping.\n\nMaybe you can write something to that end?\n\n> [...]\n> >>>> +When preparing your patch, it is important to put yourself in the\n> >>>> shoes of the maintainer.\n> >>>\n> >>> ... and in the shoes of other users and developers working with Git down\n> >>> the line who will interact with the patch later!\n> >>>\n> >>> Actually, the maintainer is one of the least important audiences for a\n> >>> commit message.  But may 'the maintainer' is a stand-in for 'the\n> >>> project' here?\n> [...]\n> > Is this not missing the point of this sentence? Those \"senior reviewers\"\n> > also put themselves into the shoes of the maintainer, because considering\n> > future readers is kind of the typical responsibility of the maintainer.\n> \n> Only in the sense that what the project does is to \"maintain\".\n> (Aside: is developing, deploying, supporting users part of\n> maintenance?  I don't actually think so!  We can call it that if you\n> like, though.).\n> \n> To put this discussion on a more practical footing: I see new\n> contributors confusing the maintainer for the audience for their\n> changes all the time, and it hurts them.  They get some useful (and\n> some unuseful, sadly) replies from the project and none from the\n> maintainer and they end up confused: does this mean that the person\n> that matters doesn't even care about my patch?\n\nAh! I really did misunderstand you. To me, that \"put yourself in the shoes\nof the maintainer\" thing was only intended to write the code in a certain\nway, not to raise expectations who would answer on the mailing list.\n\n> Clarifying that the audience is *not* the maintainer can help.\n> \n> I would not be surprised if the Git for Windows project works\n> differently: maybe you have the time, inclination, and ability to\n> respond to every contributor directly early in the process.  There's\n> nothing wrong with that, and it probably helps to make the process\n> easier for contributors with this particular confusion.  Having a\n> single person that is ready to talk to them and can stand in for what\n> the entire project needs can make their life easier.\n> \n> But that's not sustainable in the Git project.  Larger projects either\n> need multiple maintainers or a decrease in scope of the maintainer's\n> role.  Junio uses the latter, which seems fine to me.\n> \n> Even something as simple as s/maintainer/maintainers/ would make this\n> text less misleading for people contributing to upstream Git.\n\nSure. Works for me.\n\n> [...]\n> >>>> +* Make sure the commits are signed off using `git commit\n> >>>> (-s|--signoff)`. This means that you testify to be allowed to\n> >>>> contribute your changes, in particular that if you did this on\n> >>>> company time, said company approved to releasing your work as Open\n> >>>> Source under the GNU Public License v2.\n> >>>\n> >>> Can this either point to or quote the relevant legal text from\n> >>> Documentation/SubmittingPatches?  It feels dangerous (in the sense of,\n> >>> potentially leading to misunderstandings and major future pain) to ask\n> >>> people to sign off without them knowing exactly what that means.\n> >>\n> >> When you can point at an existing test in legal context, it is safer\n> >> to do so rather than attempting to \"rephrase\" it yourself (unless\n> >> you are a lawyer, of course, in which case you can assess the best\n> >> course of action yourself).\n> >\n> > Please note: Documentation/SubmittingPatches is not a legal text. At least\n> > I have not seen any accredited lawyer commenting on the validity of this\n> > document, or how much sense it makes.\n> \n> This seems to reflect a misunderstanding of how law works, at least in\n> the United States.\n> \n> I don't know how one would define what a legal text means.  I assume\n> you mean \"a contract\", and I certainly don't care to evaluate whether\n> some particular text is a contract: I am not a lawyer.\n> \n> But there are all kinds of other actions or words with legal\n> implications:\n> \n>  - promises\n>  - injuries\n>  - etc, etc, etc\n> \n> Basically anything someone ever does has potential legal implications.\n\nSo everything is a legal text.\n\n> The developer's certificate of origin has been reviewed by plenty of\n> lawyers.  You can have your own lawyers review it too if you like.\n> \n> That's all side-stepping what I was saying, which is that it's bad for\n> the project for people to be signing off without knowing what it\n> means.\n\nYes, and I agree that the link needs to be there.\n\n> [...]\n> > Even so, Git for Windows' own wiki\n> > (https://github.com/git-for-windows/git/wiki/Good-commits) links to a\n> > particular version of Documentation/SubmittingPatches so that it can\n> > specifically mark the DCO:\n> >\n> > https://github.com/git/git/blob/v2.8.1/Documentation/SubmittingPatches#L234-L286\n> \n> Great.  That's the sort of thing I'm asking for in this new\n> contributors document.  Why in the world is it useful to push back\n> against such a request?\n\nOh, I was just taking exception with the expression \"legal text\" being\napplied to something I consider a bit informal.\n\nPut another way: I indulged in veering off on a tangent. You know, like we\ndo for fun here ;-)\n\nCiao,\nDscho\n"},{"id":"341039","messageId":"20180305180109.GA28067@aiede.svl.corp.google.com","threadId":"47953","inReplyTo":"nycvar.QRO.7.76.6.1803051711210.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [RFC] Contributing to Git (on Windows)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-03-05T18:01:09Z","receivedAt":"2018-03-05T18:01:25Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Dscho,\n\nJohannes Schindelin wrote:\n> On Sat, 3 Mar 2018, Jonathan Nieder wrote:\n\n>> Hopefully the clarifications and suggestions higher in this message\n>> help.  If they don't, then I'm nervous about our ability to understand\n>> each other.\n>\n> Okay, let me state what I think the goal of this document should be:\n>\n> \tTo help Windows-based developers who want to contribute their first\n> \tpatch to the Git project.\n>\n> Whatever makes it easier to contributors and is easily explained, should\n> go in, in my opinion.\n>\n> Wishful thinking, and philosophical considerations, should probably stay\n> out.\n\nI think this conversation has gone way off the rails.\n\nI certainly share some blame for that: in\nhttps://public-inbox.org/git/20180303182723.GA76934@aiede.svl.corp.google.com/\nI let my emotions get the better of me and let my bafflement show\ninstead of focusing on my gratitude for the context and clarifications\nyou were providing.  And I am grateful for those.\n\nWhat went wrong is that I somehow turned it into a debate.  That's not\nthe point of a patch review.  After all, we have the same goals for\nthis document!  So I am happy to continue to work with Derrick Stolee\n(and anyone else interested) on whatever improvements he would like to\npursue, but I am going to bow out of the arguing with you part, if\nthat's okay.\n\nJonathan\n"},{"id":"341040","messageId":"20180305181333.GB28067@aiede.svl.corp.google.com","threadId":"47953","inReplyTo":"663f3fef-30bc-8900-6070-80ac96cccff0@gmail.com","subject":"Re: [RFC] Contributing to Git (on Windows)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-03-05T18:13:33Z","receivedAt":"2018-03-05T18:13:41Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Derrick Stolee wrote:\n>>>>> Dereck Stolee wrote:\n>\n> nit: s/Dereck/Derrick/ Is my outgoing email name misspelled, or do you have\n> a misspelled contact info for me?\n\nA manual typo.  Sorry about that.\n\n[... a bunch snipped ...]\n> I have a habit of being too loose in language around lawyer-speak. I should\n> not have attempted to summarize what \"Signed-off-by:\" means and will use\n> that helpful link for the description instead.\n\nNo worries.  I make that kind of mistake all the time but just thought\nit worth pointing out.\n\nBTW, thanks again for writing and submitting this document.  It can't\nland soon enough. :)\n\nJonathan\n"},{"id":"341042","messageId":"20180305182658.GC28067@aiede.svl.corp.google.com","threadId":"47953","inReplyTo":"nycvar.QRO.7.76.6.1803051711210.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [RFC] Contributing to Git (on Windows)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-03-05T18:26:58Z","receivedAt":"2018-03-05T18:27:06Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi again,\n\nBack on topic, some quick clarifications to tie up loose ends.\n\nAlso I want to thank you for continuing to push the project to work\nbetter (especially to work better for newcomers).  We don't seem to\nhave a habit of saying often enough how important that goal is.  Of\ncourse we'll disagree from time to time in minor ways about particular\naspects of how to change the development workflow, but the progress\nyou've already made (e.g. via tools like SubmitGit) is a huge deal.\n\n[...]\nJohannes Schindelin wrote:\n> On Sat, 3 Mar 2018, Jonathan Nieder wrote:\n\n>>  1. Approximately 1/2 of the differences you describe apply to Mac as\n>>     well as Windows.\n>\n> No. The executable bit, the native line endings, most of the issues I\n> listed do not catch macOS-based developers off guard.\n\nYeah, my wording was really sloppy.\n\nI meant that one of the differences you described half-applies to Mac\nand the rest don't apply to Mac.  I should have proofread.\n\n[...]\n> Stolee agreed in the PR to mention alternatives to Hyper-V, such as\n> VirtualBox, which would help macOS-based developers here.\n\nI have no opinion about that (maybe it will make the text too long and\noverwhelming, or maybe it will help people on balance).\n\n[...]\n> The Google gang (you & Junio included) uses Linux. Peff uses Linux. From\n> what I can see Duy, Eric and Jake use Linux. That covers already the most\n> active reviewers right there.\n\nWe are not as uniform as it may seem.  The Google gang all uses Linux\non workstations but some use Mac laptops as well.  We deal with user\nreports all the time from all three popular platforms.\n\nIIRC then Junio has a test setup that tests on Linux, FreeBSD, and\nsome other platforms.  IIRC Microsoft provides a way to run a Windows\nVM for development purposes that he could use for testing in the same\nway as he tests on FreeBSD if there are clear enough instructions for\ndoing it (hint, hint). :)\n\nOf course it's even better if there is some public shared build/test\ndashboard that we can all work together to at least keep green.\n\n[...]\n> So where is that formatter call that fixes your code?\n\nThere's \"make style\", and people still continue to work on improving\nits configuration (thanks!).\n\n[...]\n> However, VSTS is free for teams up to five, meaning that any developer can\n> register a project, and once I manage to get build definitions in shape, I\n> can make them public and anybody can test their code on the major three\n> platforms, in their personal VSTS account.\n\nThanks.  That sounds potentially useful.  (Though a shared dashboard\nthat we all keep green might be even more so.)\n\n[...]\n> So everything is a legal text.\n\nYeah.  In this context I should have instead said something like\n\"lawyer-reviewed text\".\n\n[...]\n> Put another way: I indulged in veering off on a tangent. You know, like we\n> do for fun here ;-)\n\nFeel free to call me on it when my tangents are hurting, or when\nthey're helping for that matter.  That way I have training data to\nimprove my model of how to make a good tangent. :)\n\nSincerely,\nJonathan\n"},{"id":"341051","messageId":"CAPig+cQa9B6OJwH6TB14JO8SQ2iVDdKXBBAtgz4wdtn2zRo-qw@mail.gmail.com","threadId":"47953","inReplyTo":"20180305182658.GC28067@aiede.svl.corp.google.com","subject":"Re: [RFC] Contributing to Git (on Windows)","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-03-05T18:58:37Z","receivedAt":"2018-03-05T18:58:43Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 5, 2018 at 1:26 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Johannes Schindelin wrote:\n>> The Google gang (you & Junio included) uses Linux. Peff uses Linux. From\n>> what I can see Duy, Eric and Jake use Linux. That covers already the most\n>> active reviewers right there.\n>\n> We are not as uniform as it may seem.  The Google gang all uses Linux\n> on workstations but some use Mac laptops as well.  We deal with user\n> reports all the time from all three popular platforms.\n\nAnd, Eric uses Mac, not Linux, though he does test his submissions on\nLinux and BSD VM's.\n"},{"id":"341059","messageId":"xmqqwoyq8e61.fsf@gitster-ct.c.googlers.com","threadId":"47953","inReplyTo":"nycvar.QRO.7.76.6.1803051711210.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [RFC] Contributing to Git (on Windows)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-05T20:23:34Z","receivedAt":"2018-03-05T20:23:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> > \"What if they say my code is not good enough?\"\n>> \n>> Sure, though there is something implied in what is Junio is saying\n>> that is useful for such people.\n>> \n>> It is patience.  It is the message that if you miss a portability bug,\n>> we won't be disappointed in you, and it in fact happens all the time\n>> to the best of contributors.\n>> \n>> If there's a straightforward way to convey that in the text, I agree\n>> with Junio that it's worth conveying.\n>\n> That is not how I read Junio's statement. I read it more like \"If your\n> code is flawed, we'll let you know.\"\n\nI think you are talking about this part?\n\n    In fact, portability issues in a patch originally written for a\n    platform is rather quickly discovered if the original platform\n    is more minor than the others, so while it is good advice to\n    test your ware before you make it public, singling out the\n    portability issues may not add much value.\n\nIt's more like \"Just like everybody else you are expected to be\nimperfect, but those on list can and will help spot and fix if you\nmade mistakes.  Do not worry too much about things like portability\nover to a system you usually do not work on.\"\n\nThe other side of the coin is that we are expected to be imperfect,\nso even if your code is flawed, we may not even notice, so we may\nnot let you know.  It's mutual process in which we try to help each\nother.\n\n"},{"id":"341205","messageId":"nycvar.QRO.7.76.6.1803071516070.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"47953","inReplyTo":"20180305181333.GB28067@aiede.svl.corp.google.com","subject":"Re: [RFC] Contributing to Git (on Windows)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-07T14:16:28Z","receivedAt":"2018-03-07T14:16:41Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jonathan,\n\nOn Mon, 5 Mar 2018, Jonathan Nieder wrote:\n\n> BTW, thanks again for writing and submitting this document.  It can't\n> land soon enough. :)\n\nIt landed:\n\n\thttps://github.com/git-for-windows/git/blob/master/CONTRIBUTING.md\n\nCiao,\nDscho\n"}]}