On 3/1/2018 11:44 PM, Jonathan Nieder wrote:
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 . 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?
@@ -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
so that line ending conversion takes place even if the user hasn't
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
+ The SDK uses a different credential manager, so you may still want to use
+ or normal Git Bash for interacting with your remotes. Alternatively, use
+ 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  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.
+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
+[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.
<shameless plug>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
+helper commands that test a very limited action. These commands are stored in
+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
+executable, so be sure the test helper is required.
+Read [`t/README`](https://github.com/git/git/blob/master/t/README) for more
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
+repositories or with more repetitions. The full list is available in
+Test Your Changes on Linux
+It can be important to work directly on the [core Git
+such as a recent commit into the `master` or `next` branch that has not been
+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
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 enough to catch the platform
differences, but they do catch things that you wouldn't think are
+When preparing your patch, it is important to put yourself in the shoes of the
... 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
Stand-in for "the community" yeah.
+* 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.
Thanks for the feedback.