[RFC] Contributing to Git (on Windows)

[RFC] Contributing to Git (on Windows)

[Derrick Stolee]

We (Git devs at Microsoft) have had several people start contributing to 
Git over the past few years (I'm the most-recent addition). As we 
on-boarded to Git development on our Windows machines, we collected our 
setup steps on an internal wiki page.

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.

If you've ever done Git development on a Windows machine, I would love 
to hear your feedback on this document. Any other advice you have is 
greatly appreciated.

For anyone interested, there are also a discussion about submitting 
patches upstream. The document links to Documentation/CodingGuidelines 
and Documentation/SubmittingPatches, but tries to elaborate with 
additional advice.

Thanks,
-Stolee

[1] https://github.com/git-for-windows/git/pull/1529

Re: [RFC] Contributing to Git (on Windows)

[Jonathan Nieder]

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.

<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 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

Re: [RFC] Contributing to Git (on Windows)

[Derrick Stolee]

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.
> <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 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 enough to catch the platform 
differences, but they do catch things that you wouldn't think are 
platform-specific.

>
> [...]
>> +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?

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.

+1 thanks.

>
> The rest also looks nice.  Thanks for working on this.
>
> Sincerely,
> Jonathan

Thanks for the feedback.

-Stolee

Re: [RFC] Contributing to Git (on Windows)

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> 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)

[Johannes Schindelin]

Hi,

On Fri, 2 Mar 2018, Junio C Hamano wrote:

> Jonathan Nieder <jrnieder@gmail.com> 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 on
an eager, first-time Open Source contributors is very different, though.
It is something that can turn a first-time contributor into a
one-time-only contributor.

> s/Linux/other platforms/, perhaps?

No, this advice comes straight from my personal observation that the
reviewers on the Git mailing list are Linux-centric.

And this advice served my colleagues very well, before we had set up the
automated Windows/Linux/macOS builds on our internal Pull Requests.

Plus. We need to stay reasonable. This guide is trying to guide first-time
contributors how to contribute to the Git project, with a slight bias
towards Windows-based contributors. It is very easy for them to get access
to a Linux VM: they simply use the Hyper-V Manager (or obtain VirtualBox)
and install a freely-downloadable Ubuntu .iso.

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.

And it does not make sense to suggest testing e.g. on FreeBSD:

1) there had been known breakages of our test suite on FreeBSD, and

2) historically, Linux, macOS and Windows are the most important Operating
Systems when it comes to reviews on the Git mailing list, in that order.
(I would actually disagree that this reflects the number of respective Git
users, but that is another story, and it would be very hard to obtain
trustworthy evidence either.)

> 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.

You put yourself into the shoes of the maintainer, I see, as asked for by
Stolee's document.

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?"

The document we are discussing here is intended to be useful for these
potential contributors.

To you, it may very well look like a great thing if they used a "minor"
platform (such as Windows, you clearly thought ;-)) so your review would
make their code portable enough to work even on Linux. To the first-time
contributors *I* know, it looks like a very terrifying thing. "The
maintainer hates my patches!"

So. I would rather want to be nice to contributors who have not yet
contributed code to the Git mailing list.

> >> +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.

Is this not missing the point of this sentence? Those "senior reviewers"
also put themselves into the shoes of the maintainer, because considering
future readers is kind of the typical responsibility of the maintainer.
And considering how maintainable the code is. And how correct it is. How
easy it will be to spot regressions.

So "put yourself into the shoes of the maintainer" is a pretty good advice
here, because it will automatically also catch all those other groups of
people you mentioned, without mentioning them.

> > [...]
> >> +* 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).

Please note: Documentation/SubmittingPatches is not a legal text. At least
I have not seen any accredited lawyer commenting on the validity of this
document, or how much sense it makes.

I could imagine that it would not hold up in court, given that there is no
required consent to the "terms" of Documentation/SubmittingPatches before
anybody sends any patches, and every contributor could claim that they
meant something different by "Signed-off-by:", that they had not even read
Documentation/SubmittingPatches. It *would* probably be different if the
contribution process required opening a Pull Request on GitHub and the
.github/PULL_REQUEST_TEMPLATE.md stated explicitly what we mean by the
"Signed-off-by:" line.

Even so, Git for Windows' own wiki
(https://github.com/git-for-windows/git/wiki/Good-commits) links to a
particular version of Documentation/SubmittingPatches so that it can
specifically mark the DCO:

https://github.com/git/git/blob/v2.8.1/Documentation/SubmittingPatches#L234-L286

I would probably want to update that to v2.16.2's version and link to that
from the guide, but definitely keep the current wording (as a preview of
the full Developers' Certificate of Origin).

Please note also: there is a seemingly authoritative site at
https://developercertificate.org/ but it does not even talk about
"Signed-off-by:", so we cannot really link there instead.

Ciao,
Dscho

Re: [RFC] Contributing to Git (on Windows)

[Jonathan Nieder]

Hi Dscho,

Johannes Schindelin wrote:
>> Jonathan Nieder <jrnieder@gmail.com> 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 there's a straightforward way to convey that in the text, I agree
with Junio that it's worth conveying.

[...]
>>>> +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?
[...]
> Is this not missing the point of this sentence? Those "senior reviewers"
> also put themselves into the shoes of the maintainer, because considering
> future readers is kind of the typical responsibility of the maintainer.

Only in the sense that what the project does is to "maintain".
(Aside: is developing, deploying, supporting users part of
maintenance?  I don't actually think so!  We can call it that if you
like, though.).

To put this discussion on a more practical footing: I see new
contributors confusing the maintainer for the audience for their
changes all the time, and it hurts them.  They get some useful (and
some unuseful, sadly) replies from the project and none from the
maintainer and they end up confused: does this mean that the person
that matters doesn't even care about my patch?

Clarifying that the audience is *not* the maintainer can help.

I would not be surprised if the Git for Windows project works
differently: maybe you have the time, inclination, and ability to
respond to every contributor directly early in the process.  There's
nothing wrong with that, and it probably helps to make the process
easier for contributors with this particular confusion.  Having a
single person that is ready to talk to them and can stand in for what
the entire project needs can make their life easier.

But that's not sustainable in the Git project.  Larger projects either
need multiple maintainers or a decrease in scope of the maintainer's
role.  Junio uses the latter, which seems fine to me.

Even something as simple as s/maintainer/maintainers/ would make this
text less misleading for people contributing to upstream Git.

[...]
>>>> +* 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).
>
> Please note: Documentation/SubmittingPatches is not a legal text. At least
> I have not seen any accredited lawyer commenting on the validity of this
> document, or how much sense it makes.

This seems to reflect a misunderstanding of how law works, at least in
the United States.

I don't know how one would define what a legal text means.  I assume
you mean "a contract", and I certainly don't care to evaluate whether
some particular text is a contract: I am not a lawyer.

But there are all kinds of other actions or words with legal
implications:

 - promises
 - injuries
 - etc, etc, etc

Basically anything someone ever does has potential legal implications.

The developer's certificate of origin has been reviewed by plenty of
lawyers.  You can have your own lawyers review it too if you like.

That's all side-stepping what I was saying, which is that it's bad for
the project for people to be signing off without knowing what it
means.

[...]
> Even so, Git for Windows' own wiki
> (https://github.com/git-for-windows/git/wiki/Good-commits) links to a
> particular version of Documentation/SubmittingPatches so that it can
> specifically mark the DCO:
>
> https://github.com/git/git/blob/v2.8.1/Documentation/SubmittingPatches#L234-L286

Great.  That's the sort of thing I'm asking for in this new
contributors document.  Why in the world is it useful to push back
against such a request?

Confused,
Jonathan

Re: [RFC] Contributing to Git (on Windows)

[Derrick Stolee]

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 <jrnieder@gmail.com> 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 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 there's a straightforward way to convey that in the text, I agree
> with Junio that it's worth conveying.
>
> [...]
>>>>> +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?
> [...]
>> Is this not missing the point of this sentence? Those "senior reviewers"
>> also put themselves into the shoes of the maintainer, because considering
>> future readers is kind of the typical responsibility of the maintainer.
> Only in the sense that what the project does is to "maintain".
> (Aside: is developing, deploying, supporting users part of
> maintenance?  I don't actually think so!  We can call it that if you
> like, though.).
>
> To put this discussion on a more practical footing: I see new
> contributors confusing the maintainer for the audience for their
> changes all the time, and it hurts them.  They get some useful (and
> some unuseful, sadly) replies from the project and none from the
> maintainer and they end up confused: does this mean that the person
> that matters doesn't even care about my patch?
>
> Clarifying that the audience is *not* the maintainer can help.
>
> I would not be surprised if the Git for Windows project works
> differently: maybe you have the time, inclination, and ability to
> respond to every contributor directly early in the process.  There's
> nothing wrong with that, and it probably helps to make the process
> easier for contributors with this particular confusion.  Having a
> single person that is ready to talk to them and can stand in for what
> the entire project needs can make their life easier.
>
> But that's not sustainable in the Git project.  Larger projects either
> need multiple maintainers or a decrease in scope of the maintainer's
> role.  Junio uses the latter, which seems fine to me.
>
> Even something as simple as s/maintainer/maintainers/ would make this
> text less misleading for people contributing to upstream Git.

I really like the suggestion to swap "maintainer" for "community". It's 
something I've learned by being on the list since I first wrote that 
paragraph about "the maintainer" in October. Back then, I was new to the 
list and my main way of understanding what was going on was by reading 
Junio's "What's Cooking" emails.

> [...]
>>>>> +* 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).
>> Please note: Documentation/SubmittingPatches is not a legal text. At least
>> I have not seen any accredited lawyer commenting on the validity of this
>> document, or how much sense it makes.
> This seems to reflect a misunderstanding of how law works, at least in
> the United States.
>
> I don't know how one would define what a legal text means.  I assume
> you mean "a contract", and I certainly don't care to evaluate whether
> some particular text is a contract: I am not a lawyer.
>
> But there are all kinds of other actions or words with legal
> implications:
>
>   - promises
>   - injuries
>   - etc, etc, etc
>
> Basically anything someone ever does has potential legal implications.
>
> The developer's certificate of origin has been reviewed by plenty of
> lawyers.  You can have your own lawyers review it too if you like.
>
> That's all side-stepping what I was saying, which is that it's bad for
> the project for people to be signing off without knowing what it
> means.
>
> [...]
>> Even so, Git for Windows' own wiki
>> (https://github.com/git-for-windows/git/wiki/Good-commits) links to a
>> particular version of Documentation/SubmittingPatches so that it can
>> specifically mark the DCO:
>>
>> https://github.com/git/git/blob/v2.8.1/Documentation/SubmittingPatches#L234-L286
> Great.  That's the sort of thing I'm asking for in this new
> contributors document.  Why in the world is it useful to push back
> against such a request?
>

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.

Thanks,
-Stolee

Re: [RFC] Contributing to Git (on Windows)

[Johannes Schindelin]

Hi Jonathan,

On Sat, 3 Mar 2018, Jonathan Nieder wrote:

> Johannes Schindelin wrote:
> >> Jonathan Nieder <jrnieder@gmail.com> 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 formatter call that fixes your code? Where is that
statement that says that we have a consistent code formatting, and don't
let taste override automatic formatting settings?

As long as you allow taste to rule the coding style where automated tools
could prettify code consistently, you will always have those discussions
(that I personally really dislike, as I find that they distract from my
*code*).

> [...]
> > 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.

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.

Yes, in an ideal world contributors would read this document and then
understand what it takes to contribute to this heterogenous project. But
that is impractical. Already you and me disagree on many issues. Those
disagreements do not need to be part of that document.

> [...]
> > 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.

For the moment, we have Travis CI. Since it leaves out Windows, it does
not help my job one bit, and whatever I did to get Windows testing into
our Continuous Testing (it is *not* Continuous Integration, you probably
agree with me on *that*) is but a stop-gap measure.

I am part of the organization developing Visual Studio Team Services
(VSTS), the integrated DevOps solution also used to develop Windows using
the largest repository on this planet, and as such have the privilege of
being able to use it to my heart's extent.  It has build agents for
Windows, Linux and macOS. I hope to get a bit more time soon to play with
them and get some more robust testing in place by copy/editing the build
definitions I already use for our internal PRs.

The downside is that the logs are not public.

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.

> [...]
> > 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 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."

But I like your idea of conveying in the document that you can expect
kind and patient help in improving your patches. To somehow stave off the
feeling of getting criticized, and reframing it instead to tell
contributors that this is intended as helping.

Maybe you can write something to that end?

> [...]
> >>>> +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?
> [...]
> > Is this not missing the point of this sentence? Those "senior reviewers"
> > also put themselves into the shoes of the maintainer, because considering
> > future readers is kind of the typical responsibility of the maintainer.
> 
> Only in the sense that what the project does is to "maintain".
> (Aside: is developing, deploying, supporting users part of
> maintenance?  I don't actually think so!  We can call it that if you
> like, though.).
> 
> To put this discussion on a more practical footing: I see new
> contributors confusing the maintainer for the audience for their
> changes all the time, and it hurts them.  They get some useful (and
> some unuseful, sadly) replies from the project and none from the
> maintainer and they end up confused: does this mean that the person
> that matters doesn't even care about my patch?

Ah! I really did misunderstand you. To me, that "put yourself in the shoes
of the maintainer" thing was only intended to write the code in a certain
way, not to raise expectations who would answer on the mailing list.

> Clarifying that the audience is *not* the maintainer can help.
> 
> I would not be surprised if the Git for Windows project works
> differently: maybe you have the time, inclination, and ability to
> respond to every contributor directly early in the process.  There's
> nothing wrong with that, and it probably helps to make the process
> easier for contributors with this particular confusion.  Having a
> single person that is ready to talk to them and can stand in for what
> the entire project needs can make their life easier.
> 
> But that's not sustainable in the Git project.  Larger projects either
> need multiple maintainers or a decrease in scope of the maintainer's
> role.  Junio uses the latter, which seems fine to me.
> 
> Even something as simple as s/maintainer/maintainers/ would make this
> text less misleading for people contributing to upstream Git.

Sure. Works for me.

> [...]
> >>>> +* 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).
> >
> > Please note: Documentation/SubmittingPatches is not a legal text. At least
> > I have not seen any accredited lawyer commenting on the validity of this
> > document, or how much sense it makes.
> 
> This seems to reflect a misunderstanding of how law works, at least in
> the United States.
> 
> I don't know how one would define what a legal text means.  I assume
> you mean "a contract", and I certainly don't care to evaluate whether
> some particular text is a contract: I am not a lawyer.
> 
> But there are all kinds of other actions or words with legal
> implications:
> 
>  - promises
>  - injuries
>  - etc, etc, etc
> 
> Basically anything someone ever does has potential legal implications.

So everything is a legal text.

> The developer's certificate of origin has been reviewed by plenty of
> lawyers.  You can have your own lawyers review it too if you like.
> 
> That's all side-stepping what I was saying, which is that it's bad for
> the project for people to be signing off without knowing what it
> means.

Yes, and I agree that the link needs to be there.

> [...]
> > Even so, Git for Windows' own wiki
> > (https://github.com/git-for-windows/git/wiki/Good-commits) links to a
> > particular version of Documentation/SubmittingPatches so that it can
> > specifically mark the DCO:
> >
> > https://github.com/git/git/blob/v2.8.1/Documentation/SubmittingPatches#L234-L286
> 
> Great.  That's the sort of thing I'm asking for in this new
> contributors document.  Why in the world is it useful to push back
> against such a request?

Oh, I was just taking exception with the expression "legal text" being
applied to something I consider a bit informal.

Put another way: I indulged in veering off on a tangent. You know, like we
do for fun here ;-)

Ciao,
Dscho

Re: [RFC] Contributing to Git (on Windows)

[Jonathan Nieder]

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.GA76934@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)

[Jonathan Nieder]

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)

[Jonathan Nieder]

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)

[Eric Sunshine]

On Mon, Mar 5, 2018 at 1:26 PM, Jonathan Nieder <jrnieder@gmail.com> 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)

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> 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)

[Johannes Schindelin]

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