Re: [RFC] Contributing to Git (on Windows)
Hi Jonathan, On Mon, 5 Mar 2018, Jonathan Nieder wrote: > BTW, thanks again for writing and submitting this document. It can't > land soon enough. :) It landed: https://github.com/git-for-windows/git/blob/master/CONTRIBUTING.md Ciao, Dscho
Re: [RFC] Contributing to Git (on Windows)
Johannes Schindelin writes: >> > "What if they say my code is not good enough?" >> >> Sure, though there is something implied in what is Junio is saying >> that is useful for such people. >> >> It is patience. It is the message that if you miss a portability bug, >> we won't be disappointed in you, and it in fact happens all the time >> to the best of contributors. >> >> If there's a straightforward way to convey that in the text, I agree >> with Junio that it's worth conveying. > > That is not how I read Junio's statement. I read it more like "If your > code is flawed, we'll let you know." I think you are talking about this part? In fact, portability issues in a patch originally written for a platform is rather quickly discovered if the original platform is more minor than the others, so while it is good advice to test your ware before you make it public, singling out the portability issues may not add much value. It's more like "Just like everybody else you are expected to be imperfect, but those on list can and will help spot and fix if you made mistakes. Do not worry too much about things like portability over to a system you usually do not work on." The other side of the coin is that we are expected to be imperfect, so even if your code is flawed, we may not even notice, so we may not let you know. It's mutual process in which we try to help each other.
Re: [RFC] Contributing to Git (on Windows)
On Mon, Mar 5, 2018 at 1:26 PM, Jonathan Nieder wrote: > Johannes Schindelin wrote: >> The Google gang (you & Junio included) uses Linux. Peff uses Linux. From >> what I can see Duy, Eric and Jake use Linux. That covers already the most >> active reviewers right there. > > We are not as uniform as it may seem. The Google gang all uses Linux > on workstations but some use Mac laptops as well. We deal with user > reports all the time from all three popular platforms. And, Eric uses Mac, not Linux, though he does test his submissions on Linux and BSD VM's.
Re: [RFC] Contributing to Git (on Windows)
Hi again, Back on topic, some quick clarifications to tie up loose ends. Also I want to thank you for continuing to push the project to work better (especially to work better for newcomers). We don't seem to have a habit of saying often enough how important that goal is. Of course we'll disagree from time to time in minor ways about particular aspects of how to change the development workflow, but the progress you've already made (e.g. via tools like SubmitGit) is a huge deal. [...] Johannes Schindelin wrote: > On Sat, 3 Mar 2018, Jonathan Nieder wrote: >> 1. Approximately 1/2 of the differences you describe apply to Mac as >> well as Windows. > > No. The executable bit, the native line endings, most of the issues I > listed do not catch macOS-based developers off guard. Yeah, my wording was really sloppy. I meant that one of the differences you described half-applies to Mac and the rest don't apply to Mac. I should have proofread. [...] > Stolee agreed in the PR to mention alternatives to Hyper-V, such as > VirtualBox, which would help macOS-based developers here. I have no opinion about that (maybe it will make the text too long and overwhelming, or maybe it will help people on balance). [...] > The Google gang (you & Junio included) uses Linux. Peff uses Linux. From > what I can see Duy, Eric and Jake use Linux. That covers already the most > active reviewers right there. We are not as uniform as it may seem. The Google gang all uses Linux on workstations but some use Mac laptops as well. We deal with user reports all the time from all three popular platforms. IIRC then Junio has a test setup that tests on Linux, FreeBSD, and some other platforms. IIRC Microsoft provides a way to run a Windows VM for development purposes that he could use for testing in the same way as he tests on FreeBSD if there are clear enough instructions for doing it (hint, hint). :) Of course it's even better if there is some public shared build/test dashboard that we can all work together to at least keep green. [...] > So where is that formatter call that fixes your code? There's "make style", and people still continue to work on improving its configuration (thanks!). [...] > However, VSTS is free for teams up to five, meaning that any developer can > register a project, and once I manage to get build definitions in shape, I > can make them public and anybody can test their code on the major three > platforms, in their personal VSTS account. Thanks. That sounds potentially useful. (Though a shared dashboard that we all keep green might be even more so.) [...] > So everything is a legal text. Yeah. In this context I should have instead said something like "lawyer-reviewed text". [...] > Put another way: I indulged in veering off on a tangent. You know, like we > do for fun here ;-) Feel free to call me on it when my tangents are hurting, or when they're helping for that matter. That way I have training data to improve my model of how to make a good tangent. :) Sincerely, Jonathan
Re: [RFC] Contributing to Git (on Windows)
Derrick Stolee wrote: > Dereck Stolee wrote: > > nit: s/Dereck/Derrick/ Is my outgoing email name misspelled, or do you have > a misspelled contact info for me? A manual typo. Sorry about that. [... a bunch snipped ...] > I have a habit of being too loose in language around lawyer-speak. I should > not have attempted to summarize what "Signed-off-by:" means and will use > that helpful link for the description instead. No worries. I make that kind of mistake all the time but just thought it worth pointing out. BTW, thanks again for writing and submitting this document. It can't land soon enough. :) Jonathan
Re: [RFC] Contributing to Git (on Windows)
Hi Dscho, Johannes Schindelin wrote: > On Sat, 3 Mar 2018, Jonathan Nieder wrote: >> Hopefully the clarifications and suggestions higher in this message >> help. If they don't, then I'm nervous about our ability to understand >> each other. > > Okay, let me state what I think the goal of this document should be: > > To help Windows-based developers who want to contribute their first > patch to the Git project. > > Whatever makes it easier to contributors and is easily explained, should > go in, in my opinion. > > Wishful thinking, and philosophical considerations, should probably stay > out. I think this conversation has gone way off the rails. I certainly share some blame for that: in https://public-inbox.org/git/20180303182723.ga76...@aiede.svl.corp.google.com/ I let my emotions get the better of me and let my bafflement show instead of focusing on my gratitude for the context and clarifications you were providing. And I am grateful for those. What went wrong is that I somehow turned it into a debate. That's not the point of a patch review. After all, we have the same goals for this document! So I am happy to continue to work with Derrick Stolee (and anyone else interested) on whatever improvements he would like to pursue, but I am going to bow out of the arguing with you part, if that's okay. Jonathan
Re: [RFC] Contributing to Git (on Windows)
Hi Jonathan, On Sat, 3 Mar 2018, Jonathan Nieder wrote: > Johannes Schindelin wrote: > >> Jonathan Nieder writes: > >>> Dereck Stolee wrote: > > +Test Your Changes on Linux > +-- > + > +It can be important to work directly on the [core Git > codebase](https://github.com/git/git), +such as a recent commit into > the `master` or `next` branch that has not been incorporated +into > Git for Windows. Also, it can help to run functional and performance > tests on your +code in Linux before submitting patches to the > Linux-focused mailing list. > >>> > >>> I'm surprised at this advice. Does it actually come up? > > > > Yes. > > > > I personally set up the automated builds on Windows, Linux and macOS for > > our team, and as a rule we always open an internal Pull Request on our > > topic branches as we develop them, and you would probably not believe the > > number of issues we caught before sending the patches to this list. Issues > > including > [nice list snipped] > > Thanks for explaining. I still am going to push back on the wording > here, and here is why: > > 1. Approximately 1/2 of the differences you describe apply to Mac as > well as Windows. No. The executable bit, the native line endings, most of the issues I listed do not catch macOS-based developers off guard. > The advice certainly does not apply on Mac. That is true. Stolee agreed in the PR to mention alternatives to Hyper-V, such as VirtualBox, which would help macOS-based developers here. > You might object: Mac readers would not be reading this text! But > that is not how humans work: if project documentation (e.g. the > CONTRIBUTING.md on GitHub!) says that the project is Linux-focused > and if you don't test on Linux then you might as well not bother, > then people are going to believe it. I thought the document encourages to test on Linux. It does not claim that you can forget about getting your patches accepted if you don't test on Linux. > 2. It is not unusual for Linux users to make portability mistakes that > are quickly pointed out on list. If anything, the advice to test on > other platforms should apply to contributors on Linux even more. > This happens especially often to new contributors, who sometimes use > bashisms, etc that get quickly pointed out. True. Maybe this document is not for Linux-based developers, then. I might over-generalize here, but in my experience, Windows-based developers were particularly uneasy about getting what they perceived as criticized, and for those developers it is that I want tools to tell them what is wrong. It makes it much easier to accept. > 3. I do not *want* Git to be a Linux-focused project; I want the code > to perform well on all popular platforms and for people not to be > afraid to make it so. Me, too. Realistically, though, way more than half of the reviewers on the Git mailing list use Linux, and rarely switch to any other OS even for testing. The Google gang (you & Junio included) uses Linux. Peff uses Linux. From what I can see Duy, Eric and Jake use Linux. That covers already the most active reviewers right there. So in order to get patches accepted (and that is the goal of a contributor), chances can be improved by making stuff work on Linux. > If the docs say that all we care about is Linux, then people are > likely to be too scared to do the necessary and valuable work of > making it work well on Mac, Windows, etc. The actual mix of > contributors doesn't bear it out anyway: a large number of > contributors are already on Mac or Windows. This document targets Windows-based developers. So they will already have made sure that it works well on Windows. Also, putting the burden of testing portability on first-time contributors is really what will scare them away. So don't pretend that you want to avoid scaring them if you talk about portability. > Fortunately this is pretty straightforward to fix in the doc: it could > say something like "to the multi-platform focused mailing list", for > example. You know what? I'd rather not. Seriously. I am already uneasy with suggesting to install a full-fledged Linux VM, but I think it will ultimately help contributors *not* to get scared away already by the first volley of reviewer comments. > [...] > > To my chagrin, this idea of making most of the boring stuff (and I include > > formatting in that category, but I am probably special in that respect) as > > automatable, and as little of an issue for review as possible, leaving > > most brain cycles to work on the correctness of the patches instead, did > > not really receive a lot of traction on this mailing list. > > Huh? I'm confident that this is a pretty widely supported idea within > the Git project. > > I get the feeling you must have misread something or misinterpreted a > different response. So where is that formatte
Re: [RFC] Contributing to Git (on Windows)
I really appreciate the feedback on this document, Jonathan. On 3/3/2018 1:27 PM, Jonathan Nieder wrote: Hi Dscho, Johannes Schindelin wrote: Jonathan Nieder writes: Dereck Stolee wrote: nit: s/Dereck/Derrick/ Is my outgoing email name misspelled, or do you have a misspelled contact info for me? +Test Your Changes on Linux +-- + +It can be important to work directly on the [core Git codebase](https://github.com/git/git), +such as a recent commit into the `master` or `next` branch that has not been incorporated +into Git for Windows. Also, it can help to run functional and performance tests on your +code in Linux before submitting patches to the Linux-focused mailing list. I'm surprised at this advice. Does it actually come up? Yes. I personally set up the automated builds on Windows, Linux and macOS for our team, and as a rule we always open an internal Pull Request on our topic branches as we develop them, and you would probably not believe the number of issues we caught before sending the patches to this list. Issues including [nice list snipped] Thanks for explaining. I still am going to push back on the wording here, and here is why: 1. Approximately 1/2 of the differences you describe apply to Mac as well as Windows. The advice certainly does not apply on Mac. You might object: Mac readers would not be reading this text! But that is not how humans work: if project documentation (e.g. the CONTRIBUTING.md on GitHub!) says that the project is Linux-focused and if you don't test on Linux then you might as well not bother, then people are going to believe it. 2. It is not unusual for Linux users to make portability mistakes that are quickly pointed out on list. If anything, the advice to test on other platforms should apply to contributors on Linux even more. This happens especially often to new contributors, who sometimes use bashisms, etc that get quickly pointed out. 3. I do not *want* Git to be a Linux-focused project; I want the code to perform well on all popular platforms and for people not to be afraid to make it so. If the docs say that all we care about is Linux, then people are likely to be too scared to do the necessary and valuable work of making it work well on Mac, Windows, etc. The actual mix of contributors doesn't bear it out anyway: a large number of contributors are already on Mac or Windows. Fortunately this is pretty straightforward to fix in the doc: it could say something like "to the multi-platform focused mailing list", for example. I like the "multi-platform" wording, and I'll try to use it as often as possible. I'll keep the Linux instructions because it is free to set up a Linux environment and testing in Windows and Linux will catch most of the cross-platform issues. [...] To my chagrin, this idea of making most of the boring stuff (and I include formatting in that category, but I am probably special in that respect) as automatable, and as little of an issue for review as possible, leaving most brain cycles to work on the correctness of the patches instead, did not really receive a lot of traction on this mailing list. Huh? I'm confident that this is a pretty widely supported idea within the Git project. I get the feeling you must have misread something or misinterpreted a different response. [...] No, this advice comes straight from my personal observation that the reviewers on the Git mailing list are Linux-centric. Hopefully the clarifications and suggestions higher in this message help. If they don't, then I'm nervous about our ability to understand each other. [...] Now, how reasonable do I think it is to ask those contributors to purchase an Apple machine to test their patches on macOS (you cannot just download an .iso nor would it be legal to run a macOS VM on anything but Apple hardware)? You probably guessed my answer: not at all. Agreed, this is something that needs to be automated (and not via a CONTRIBUTING.md file). As a stopgap, having a section in the contribution instructions about testing using Windows's Linux subsystem is a valuable thing, and thanks for that; I never meant to imply otherwise. [...] On Fri, 2 Mar 2018, Junio C Hamano wrote: In fact, portability issues in a patch originally written for a platform is rather quickly discovered if the original platform is more minor than the others, so while it is good advice to test your ware before you make it public, singling out the portability issues may not add much value. The fewer number of obvious bugs remaining, the fewer number of iterations it would take for a series to get in a good shape. [...] For you, Junio, however, the task *now* is to put yourself into the shoes of a Computer Science student in their 2nd year who wants to become an Open Source contributor and is a little afraid to talk directly to "the core committers", and qu
Re: [RFC] Contributing to Git (on Windows)
Hi Dscho, Johannes Schindelin wrote: >> Jonathan Nieder writes: >>> Dereck Stolee wrote: +Test Your Changes on Linux +-- + +It can be important to work directly on the [core Git codebase](https://github.com/git/git), +such as a recent commit into the `master` or `next` branch that has not been incorporated +into Git for Windows. Also, it can help to run functional and performance tests on your +code in Linux before submitting patches to the Linux-focused mailing list. >>> >>> I'm surprised at this advice. Does it actually come up? > > Yes. > > I personally set up the automated builds on Windows, Linux and macOS for > our team, and as a rule we always open an internal Pull Request on our > topic branches as we develop them, and you would probably not believe the > number of issues we caught before sending the patches to this list. Issues > including [nice list snipped] Thanks for explaining. I still am going to push back on the wording here, and here is why: 1. Approximately 1/2 of the differences you describe apply to Mac as well as Windows. The advice certainly does not apply on Mac. You might object: Mac readers would not be reading this text! But that is not how humans work: if project documentation (e.g. the CONTRIBUTING.md on GitHub!) says that the project is Linux-focused and if you don't test on Linux then you might as well not bother, then people are going to believe it. 2. It is not unusual for Linux users to make portability mistakes that are quickly pointed out on list. If anything, the advice to test on other platforms should apply to contributors on Linux even more. This happens especially often to new contributors, who sometimes use bashisms, etc that get quickly pointed out. 3. I do not *want* Git to be a Linux-focused project; I want the code to perform well on all popular platforms and for people not to be afraid to make it so. If the docs say that all we care about is Linux, then people are likely to be too scared to do the necessary and valuable work of making it work well on Mac, Windows, etc. The actual mix of contributors doesn't bear it out anyway: a large number of contributors are already on Mac or Windows. Fortunately this is pretty straightforward to fix in the doc: it could say something like "to the multi-platform focused mailing list", for example. [...] > To my chagrin, this idea of making most of the boring stuff (and I include > formatting in that category, but I am probably special in that respect) as > automatable, and as little of an issue for review as possible, leaving > most brain cycles to work on the correctness of the patches instead, did > not really receive a lot of traction on this mailing list. Huh? I'm confident that this is a pretty widely supported idea within the Git project. I get the feeling you must have misread something or misinterpreted a different response. [...] > No, this advice comes straight from my personal observation that the > reviewers on the Git mailing list are Linux-centric. Hopefully the clarifications and suggestions higher in this message help. If they don't, then I'm nervous about our ability to understand each other. [...] > Now, how reasonable do I think it is to ask those contributors to purchase > an Apple machine to test their patches on macOS (you cannot just download > an .iso nor would it be legal to run a macOS VM on anything but Apple > hardware)? You probably guessed my answer: not at all. Agreed, this is something that needs to be automated (and not via a CONTRIBUTING.md file). As a stopgap, having a section in the contribution instructions about testing using Windows's Linux subsystem is a valuable thing, and thanks for that; I never meant to imply otherwise. [...] > On Fri, 2 Mar 2018, Junio C Hamano wrote: >> In fact, portability issues in a patch originally written for a platform >> is rather quickly discovered if the original platform is more minor than >> the others, so while it is good advice to test your ware before you make >> it public, singling out the portability issues may not add much value. >> The fewer number of obvious bugs remaining, the fewer number of >> iterations it would take for a series to get in a good shape. [...] > For you, Junio, however, the task *now* is to put yourself into the shoes > of a Computer Science student in their 2nd year who wants to become an > Open Source contributor and is a little afraid to talk directly to "the > core committers", and quite scared what negative feedback they might > receive. > > "What if they say my code is not good enough?" Sure, though there is something implied in what is Junio is saying that is useful for such people. It is patience. It is the message that if you miss a portability bug, we won't be disappointed in you, and it in fact happens all the time to the best of contributors. If
Re: [RFC] Contributing to Git (on Windows)
Hi, On Fri, 2 Mar 2018, Junio C Hamano wrote: > Jonathan Nieder writes: > > >> +Test Your Changes on Linux > >> +-- > >> + > >> +It can be important to work directly on the [core Git > >> codebase](https://github.com/git/git), +such as a recent commit into > >> the `master` or `next` branch that has not been incorporated +into > >> Git for Windows. Also, it can help to run functional and performance > >> tests on your +code in Linux before submitting patches to the > >> Linux-focused mailing list. > > > > I'm surprised at this advice. Does it actually come up? Yes. I personally set up the automated builds on Windows, Linux and macOS for our team, and as a rule we always open an internal Pull Request on our topic branches as we develop them, and you would probably not believe the number of issues we caught before sending the patches to this list. Issues including - scripts not being marked executable (on Windows, we don't care, but you Linux folks do), - use of symbols that are only available on Windows, but that were not guarded by appropriate `#ifdef`s, - printf formats available only on Windows, - EOL_NATIVE differences, - differences between BSD and GNU tools, such as the white-space in the output of `wc`, the syntactic differences of `sed`'s `-i` option, different options of the `stat` utility to do the same thing, etc (granted, this would not be caught when only testing on Windows and Linux, as both development environments would only use the GNU variety) Of course, these builds also catch problems by setting DEVELOPER=1 which developers might have forgotten to specify when building locally (it really is too easy to forget). > > When I was on Mac I never worried about this, nor when I was on > > Windows. I commend your boldness. Of course, you are an old-timer. These instructions are intended for new-timers, even first-timers, who may very well be very scared of reviews criticizing them for, say, not marking a new test script as executable. If you ever spoke to potential contributors who decided not to contribute after all, and you dig a little deeper, you might get the same impression as I got: they are (rightfully) scared of receiving answers with tons of comments what they did wrong. Most contributors seem to prefer to be "criticized" by tools, preferably even during the build process, where they can fix things before anybody sees their patches. This is, for example, the reason why the cURL project has this really nice `lib/checksrc.pl` script that they recommend you run to verify that the code you wrote abides by a few of their source code conventions they have. Developers run it, see what is wrong, fix it, and no human judges them. They submit the code, and the discussions revolve about how to improve the patch to cover more cases or some such. For that same reason, namely to make it less harsh for new contributors, I also tried to set up an automated job that tries to reformat patches using `clang-format` (without forcing any contributor to try to set up a bleeding-edge CLang, which can get very involved, very quickly) and pushes an updated topic branch if changes were needed. Sadly, this job is broken because I seem to be unable to get the `apt-get` commands right to get CLang's nightly builds (this job runs on Linux). To my chagrin, this idea of making most of the boring stuff (and I include formatting in that category, but I am probably special in that respect) as automatable, and as little of an issue for review as possible, leaving most brain cycles to work on the correctness of the patches instead, did not really receive a lot of traction on this mailing list. We still have no satisfactory automated way to check contributors' patches against our (quite fuzzy) coding style. Until that happens, contributors will be constantly faced with reviews about overly long lines, about grammar issues, about commit messages missing Signed-off-by: lines, about commit messages that are too short, etc. Those are all comments that not exactly make contributors feel welcome. And that comments also distract from more interesting use of reviewers' time (such as suggesting helper functions or data structures that the contributor could use to simplify the patches). But one thing that we *can* recommend to contributors (in particular Windows-based ones) is to verify that their patches work also on Linux, so that they can avoid receiving comments about that class of avoidable issues. > > I'm personally happy to review patches that haven't been tested on > > Linux, though it's of course even nicer if the patch author mentions > > what platforms they've tested on. Jonathan, I don't want this to sound harsh... but... this contributors' guide cares a little more about the feelings of the contributors than yours? ;-) Seriously again, you might not mind pointing out that a newly-added test case is not marked executable. The emotional effect of such a comment
Re: [RFC] Contributing to Git (on Windows)
Jonathan Nieder writes: >> +When adding a helper, be sure to add a line to `t/Makefile` and to the >> `.gitignore` for the >> +binary file you add. The Git community prefers functional tests using the >> full `git` >> +executable, so be sure the test helper is required. > > Maybe s/low-level/unit/? Yup. Also "be sure the test helper is required" at the end did not click until I read it twice and realized it wanted to say that addition of new test helper executable for function level unit testing is discouraged and should be limited to cases where it is absolutely necessary. >> +Test Your Changes on Linux >> +-- >> + >> +It can be important to work directly on the [core Git >> codebase](https://github.com/git/git), >> +such as a recent commit into the `master` or `next` branch that has not >> been incorporated >> +into Git for Windows. Also, it can help to run functional and performance >> tests on your >> +code in Linux before submitting patches to the Linux-focused mailing list. > > I'm surprised at this advice. Does it actually come up? When I was > on Mac I never worried about this, nor when I was on Windows. > > I'm personally happy to review patches that haven't been tested on > Linux, though it's of course even nicer if the patch author mentions > what platforms they've tested on. s/Linux/other platforms/, perhaps? In fact, portability issues in a patch originally written for a platform is rather quickly discovered if the original platform is more minor than the others, so while it is good advice to test your ware before you make it public, singling out the portability issues may not add much value. The fewer number of obvious bugs remaining, the fewer number of iterations it would take for a series to get in a good shape. >> +When preparing your patch, it is important to put yourself in the shoes of >> the maintainer. > > ... and in the shoes of other users and developers working with Git down > the line who will interact with the patch later! > > Actually, the maintainer is one of the least important audiences for a > commit message. But may 'the maintainer' is a stand-in for 'the > project' here? I tend to agree. The reviewers all comment on the proposed log messages and code changes to help making the patch understandable by future readers (i.e. the most important audicences). The maintainer and senior reviewers may happen to be good at putting themselves in the shoes of future readers to spot potential problems than other reviewers can, simply because they have been working on the project longer than others. But we should stress that the patch should be written for future readers of the code and history. > [...] >> +* Make sure the commits are signed off using `git commit (-s|--signoff)`. >> This means that you >> + testify to be allowed to contribute your changes, in particular that if >> you did this on company >> + time, said company approved to releasing your work as Open Source under >> the GNU Public License v2. > > Can this either point to or quote the relevant legal text from > Documentation/SubmittingPatches? It feels dangerous (in the sense of, > potentially leading to misunderstandings and major future pain) to ask > people to sign off without them knowing exactly what that means. When you can point at an existing test in legal context, it is safer to do so rather than attempting to "rephrase" it yourself (unless you are a lawyer, of course, in which case you can assess the best course of action yourself).
Re: [RFC] Contributing to Git (on Windows)
On 3/1/2018 11:44 PM, Jonathan Nieder wrote: Hi, Derrick Stolee wrote: Now, we'd like to make that document publicly available. These steps are focused on a Windows user, so we propose putting them in the git-for-windows/git repo under CONTRIBUTING.md. I have a pull request open for feedback [1]. I'll read comments on that PR or in this thread. Thanks! I wonder if we can put this in the standard Documentation/ directory as well. E.g. the Windows CONTRIBUTING.md could say say "See Documentation/Contributing-On-Windows.md" so that the bulk of the text would not have to be git-for-windows specific. That's a good idea. After this review stabilizes, I'll send a patch to add the windows-specific instructions as you recommend. What precedent do we have for referencing forks like git-for-windows/git? [...] +++ b/CONTRIBUTING.md @@ -0,0 +1,401 @@ +How to Contribute to Git for Windows + Would it make sense for this to be checked in with LF instead of CRLF line endings, so that clones use the user's chosen / platform native line ending? The .gitattributes file could include /CONTRIBUTING.md text so that line ending conversion takes place even if the user hasn't enabled core.autocrlf. Oops! I turned off the CRLF munging in my repo because I had an issue with a script somewhere, but forgot to turn it back on again. Thanks for checking this. [...] +The SDK uses a different credential manager, so you may still want to use Visual Studio +or normal Git Bash for interacting with your remotes. Alternatively, use SSH rather +than HTTPS and avoid credential manager problems. Where can I read more about the problems in question? I'll try to see if we can elaborate here. The Git for Windows client ships with Git Credential Manager for Windows [1] but the SDK does not. At the very least, credential interactions are not the same and you do not have access to the credentials stored in Windows Credential Manager. [1] https://github.com/Microsoft/Git-Credential-Manager-for-Windows [...] +Many new contributors ask: What should I start working on? + +One way to win big with the maintainer of an open-source project is to look at the +[issues page](https://github.com/git-for-windows/git/issues) and see if there are any issues that +you can fix quickly, or if anything catches your eye. You can also look at https://crbug.com/git for non Windows-specific issues. And you can look at recent user questions on the mailing list: https://public-inbox.org/git [...] +If you are adding new functionality, you may need to create low-level tests by creating +helper commands that test a very limited action. These commands are stored in `t/helpers`. +When adding a helper, be sure to add a line to `t/Makefile` and to the `.gitignore` for the +binary file you add. The Git community prefers functional tests using the full `git` +executable, so be sure the test helper is required. Maybe s/low-level/unit/? [...] +Read [`t/README`](https://github.com/git/git/blob/master/t/README) for more details. Forgive my ignorance: does github flavored markdown allow relative links? (I.e., could this say [`t/README`](t/README)?) [...] +You can also set certain environment variables to help test the performance on different +repositories or with more repetitions. The full list is available in +[the `t/perf/README` file](https://github.com/git/git/blob/master/t/perf/README), Likewise. [...] +Test Your Changes on Linux +-- + +It can be important to work directly on the [core Git codebase](https://github.com/git/git), +such as a recent commit into the `master` or `next` branch that has not been incorporated +into Git for Windows. Also, it can help to run functional and performance tests on your +code in Linux before submitting patches to the Linux-focused mailing list. I'm surprised at this advice. Does it actually come up? When I was on Mac I never worried about this, nor when I was on Windows. I'm personally happy to review patches that haven't been tested on Linux, though it's of course even nicer if the patch author mentions what platforms they've tested on. Maybe this can be reframed to refer specifically to cases where you've modified some Linux platform-specific code (e.g. to add a new feature to run-command.c)? I recently had a bug in my local copy of the commit-graph patch that was only caught by our Mac OSX automated builds: I was passing the edge-value for a parent into a lookup to get an octopus edge from the graph, but I forgot to drop the most-significant bit. This cast the "uint32_t" silently into an "int" but since we multiplied by 4 somehow the Windows and Linux compilers turned this into a left-shift while Mac did not and failed during my test. Now this is an example of something that probably would have been caught in review, but stuff gets through. The Windows/Linux boundary is usually enoug
Re: [RFC] Contributing to Git (on Windows)
Hi, Derrick Stolee wrote: > Now, we'd like to make that document publicly available. These steps are > focused on a Windows user, so we propose putting them in the > git-for-windows/git repo under CONTRIBUTING.md. I have a pull request open > for feedback [1]. I'll read comments on that PR or in this thread. Thanks! I wonder if we can put this in the standard Documentation/ directory as well. E.g. the Windows CONTRIBUTING.md could say say "See Documentation/Contributing-On-Windows.md" so that the bulk of the text would not have to be git-for-windows specific. [...] > +++ b/CONTRIBUTING.md > @@ -0,0 +1,401 @@ > +How to Contribute to Git for Windows > + Would it make sense for this to be checked in with LF instead of CRLF line endings, so that clones use the user's chosen / platform native line ending? The .gitattributes file could include /CONTRIBUTING.md text so that line ending conversion takes place even if the user hasn't enabled core.autocrlf. [...] > +The SDK uses a different credential manager, so you may still want to > use Visual Studio > +or normal Git Bash for interacting with your remotes. Alternatively, > use SSH rather > +than HTTPS and avoid credential manager problems. Where can I read more about the problems in question? [...] > +Many new contributors ask: What should I start working on? > + > +One way to win big with the maintainer of an open-source project is to look > at the > +[issues page](https://github.com/git-for-windows/git/issues) and see if > there are any issues that > +you can fix quickly, or if anything catches your eye. You can also look at https://crbug.com/git for non Windows-specific issues. And you can look at recent user questions on the mailing list: https://public-inbox.org/git [...] > +If you are adding new functionality, you may need to create low-level tests > by creating > +helper commands that test a very limited action. These commands are stored > in `t/helpers`. > +When adding a helper, be sure to add a line to `t/Makefile` and to the > `.gitignore` for the > +binary file you add. The Git community prefers functional tests using the > full `git` > +executable, so be sure the test helper is required. Maybe s/low-level/unit/? [...] > +Read [`t/README`](https://github.com/git/git/blob/master/t/README) for more > details. Forgive my ignorance: does github flavored markdown allow relative links? (I.e., could this say [`t/README`](t/README)?) [...] > +You can also set certain environment variables to help test the performance > on different > +repositories or with more repetitions. The full list is available in > +[the `t/perf/README` > file](https://github.com/git/git/blob/master/t/perf/README), Likewise. [...] > +Test Your Changes on Linux > +-- > + > +It can be important to work directly on the [core Git > codebase](https://github.com/git/git), > +such as a recent commit into the `master` or `next` branch that has not been > incorporated > +into Git for Windows. Also, it can help to run functional and performance > tests on your > +code in Linux before submitting patches to the Linux-focused mailing list. I'm surprised at this advice. Does it actually come up? When I was on Mac I never worried about this, nor when I was on Windows. I'm personally happy to review patches that haven't been tested on Linux, though it's of course even nicer if the patch author mentions what platforms they've tested on. Maybe this can be reframed to refer specifically to cases where you've modified some Linux platform-specific code (e.g. to add a new feature to run-command.c)? [...] > +When preparing your patch, it is important to put yourself in the shoes of > the maintainer. ... and in the shoes of other users and developers working with Git down the line who will interact with the patch later! Actually, the maintainer is one of the least important audiences for a commit message. But may 'the maintainer' is a stand-in for 'the project' here? [...] > +* Make sure the commits are signed off using `git commit (-s|--signoff)`. > This means that you > + testify to be allowed to contribute your changes, in particular that if > you did this on company > + time, said company approved to releasing your work as Open Source under > the GNU Public License v2. Can this either point to or quote the relevant legal text from Documentation/SubmittingPatches? It feels dangerous (in the sense of, potentially leading to misunderstandings and major future pain) to ask people to sign off without them knowing exactly what that means. The rest also looks nice. Thanks for working on this. Sincerely, Jonathan
