[PATCH v1] Documentation: add setup instructions for Travis CI

[PATCH v1] Documentation: add setup instructions for Travis CI

[larsxschneider]

From: Lars Schneider <larsxschneider@gmail.com>

Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
 Documentation/SubmittingPatches | 39 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 98fc4cc..79e9b33 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -63,10 +63,43 @@ t/README for guidance.
 When adding a new feature, make sure that you have new tests to show
 the feature triggers the new behaviour when it should, and to show the
 feature does not trigger when it shouldn't.  Also make sure that the
-test suite passes after your commit.  Do not forget to update the
-documentation to describe the updated behaviour.
+test suite passes after your commit.
 
-Speaking of the documentation, it is currently a liberal mixture of US
+We recommend to use our CI infrastructure to ensure your new feature
+passes all existing tests as well as your new tests on Linux, Mac, and
+(hopefully soon) Windows.  Follow these steps for the initial setup:
+
+ (1) Sign in to GitHub: https://github.com
+     Please sign up first if you haven't already, it's free.
+
+ (2) Fork https://github.com/git/git to your GitHub account
+     Details: https://help.github.com/articles/fork-a-repo/
+
+ (3) Go to Travis CI: https://travis-ci.org
+
+ (4) Press the "Sign in with GitHub" button
+
+ (5) Grant Travis CI permissions to access your GitHub account
+     Details: https://docs.travis-ci.com/user/github-oauth-scopes
+
+ (6) Go to your Travis CI profile page: https://travis-ci.org/profile
+
+ (7) Enable Travis CI builds for your Git fork
+
+After the initial setup you can push your new feature branches to your
+Git fork on GitHub and check if they pass all tests here:
+https://travis-ci.org/<Your GitHub handle>/git/branches
+
+If they don't pass then they are marked "red". If that happens then
+click on the failing Travis CI job and scroll all the way down in the
+log. Find the line "<-- Click here to see detailed test output!" and
+click on the triangle next to the log line number to expand the detailed
+test output (example: https://travis-ci.org/git/git/jobs/122676187).
+Fix the problem and push an updated commit to your branch to ensure
+all tests pass.
+
+Do not forget to update the documentation to describe the updated
+behaviour of your new feature. It is currently a liberal mixture of US
 and UK English norms for spelling and grammar, which is somewhat
 unfortunate.  A huge patch that touches the files all over the place
 only to correct the inconsistency is not welcome, though.  Potential
-- 
2.5.1

Re: [PATCH v1] Documentation: add setup instructions for Travis CI

[Junio C Hamano]

larsxschneider@gmail.com writes:

> From: Lars Schneider <larsxschneider@gmail.com>
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
>  Documentation/SubmittingPatches | 39 ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 98fc4cc..79e9b33 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -63,10 +63,43 @@ t/README for guidance.
>  When adding a new feature, make sure that you have new tests to show
>  the feature triggers the new behaviour when it should, and to show the
>  feature does not trigger when it shouldn't.  Also make sure that the
> +test suite passes after your commit.

This is not a new issue, but it sounds as if you do not have to test
if you are not doing a new shiny toy.  Perhaps we should rephrase
the last sentence a bit.

	After any code change, make sure that the entire test suite
	passes.  After any documentation change, make sure that the
	resulting documentation set formats well.

By the way, can you teach our Travis thing to check for the "make
doc" failures?

> +We recommend to use our CI infrastructure to ensure your new feature
> +passes all existing tests as well as your new tests on Linux, Mac, and
> +(hopefully soon) Windows.  Follow these steps for the initial setup:
> +
> + (1) Sign in to GitHub: https://github.com
> +     Please sign up first if you haven't already, it's free.

Three issues:

 * None of the things utilized by the reader of this section looks
   like "our" infrastructure to me.

 * The above makes it sound as if we recommend everybody to become
   GitHub customer, which is not really the case.

 * This is overly long and deserves to be its own separate section,
   just like we have MUA specific hints, this is GitHub-Travis
   specific hints.

How about just saying

	If you have an account at GitHub (and you can get one for
	free to work on open source projects), you can use their
	Travis CI integration to test your changes on Linux, Mac,
	and (hopefully soon) Windows.  See GitHub-Travis CI hints
	section for details.
	
here, create a "GitHub-Travis CI hints" section just before "MUA
specific hints" section, and move these numbered entries and the two
paragraphs that follow to the new section.  As the introductory text
for the new section itself, it may make sense to repeat a rephrased
version of the above there, e.g.

	--------------------------------------------------
        GitHub-Travis CI hints

	With an account at GitHub (you can get one for free to work
	on open source projects), you can use Travis CI to test your
	changes on Linux, Mac, and (hopefully soon) Windows.

	Follow these steps for the initial setup:

	(1) ...
        

I'd mildly prefer to leave "Please sign up first" line out
of the first entry.

> + ...
> + (7) Enable Travis CI builds for your Git fork
> +
> +After the initial setup you can push your new feature branches to your
> +Git fork on GitHub and check if they pass all tests here:
> +https://travis-ci.org/<Your GitHub handle>/git/branches
> +
> +If they don't pass then they are marked "red". If that happens then
> +click on the failing Travis CI job and scroll all the way down in the
> +log. Find the line "<-- Click here to see detailed test output!" and
> +click on the triangle next to the log line number to expand the detailed
> +test output (example: https://travis-ci.org/git/git/jobs/122676187).
> +Fix the problem and push an updated commit to your branch to ensure
> +all tests pass.
> +
> +Do not forget to update the documentation to describe the updated
> +behaviour of your new feature. It is currently a liberal mixture of US
>  and UK English norms for spelling and grammar, which is somewhat
>  unfortunate.  A huge patch that touches the files all over the place
>  only to correct the inconsistency is not welcome, though.  Potential

Re: [PATCH v1] Documentation: add setup instructions for Travis CI

[Stefan Beller]

On Wed, Apr 13, 2016 at 10:39 AM, Junio C Hamano <gitster@pobox.com> wrote:

> here, create a "GitHub-Travis CI hints" section just before "MUA
> specific hints" section,

Somebody (I think it was you, Lars?) at GitMerge suggested to break
up the SubmittingPatches document into more than one, i.e.
the MUA hints and the Github-Travis hints could become their own documents,
and the SubmittingPatches could just contain the bare essentials.

(The file itself could also be renamed to SubmittingChanges eventually,
as the interface to the submitgit app allows you to push commits and
then transfer these to the mailing list. So while there are still patches
on the receiving end, the last contact with the change was done via
git commit/push hopefully. I dunno.)

Re: [PATCH v1] Documentation: add setup instructions for Travis CI

[Matthieu Moy]

Stefan Beller <sbeller@google.com> writes:

> On Wed, Apr 13, 2016 at 10:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> here, create a "GitHub-Travis CI hints" section just before "MUA
>> specific hints" section,
>
> Somebody (I think it was you, Lars?) at GitMerge suggested to break
> up the SubmittingPatches document into more than one, i.e.
> the MUA hints and the Github-Travis hints could become their own documents,
> and the SubmittingPatches could just contain the bare essentials.

I didn't see it on-list, but there's a PR doing that here:

https://github.com/git/git/pull/223

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v1] Documentation: add setup instructions for Travis CI

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Stefan Beller <sbeller@google.com> writes:
>
>> On Wed, Apr 13, 2016 at 10:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>>> here, create a "GitHub-Travis CI hints" section just before "MUA
>>> specific hints" section,
>>
>> Somebody (I think it was you, Lars?) at GitMerge suggested to break
>> up the SubmittingPatches document into more than one, i.e.
>> the MUA hints and the Github-Travis hints could become their own documents,
>> and the SubmittingPatches could just contain the bare essentials.
>
> I didn't see it on-list, but there's a PR doing that here:
>
> https://github.com/git/git/pull/223

I guess submitGit did not work well there ;-)?

To save one round-trip, I do not think it is unreasonable to treat
submitGit as the first-class MUA that sits next to other recommended
ways to send the patches to the list.  It is a rough equivalent to
"format-patch and then send-email".  As long as what appears on list
is the authoritative and final form of contribution, those on the
receiving end should not (and would not) care how the patch e-mail
was prepared and sent.

Re: [PATCH v1] Documentation: add setup instructions for Travis CI

[Lars Schneider]

On 13 Apr 2016, at 19:39, Junio C Hamano <gitster@pobox.com> wrote:

> larsxschneider@gmail.com writes:
> 
>> From: Lars Schneider <larsxschneider@gmail.com>
>> 
>> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
>> ---
>> Documentation/SubmittingPatches | 39 ++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 36 insertions(+), 3 deletions(-)
>> 
>> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
>> index 98fc4cc..79e9b33 100644
>> --- a/Documentation/SubmittingPatches
>> +++ b/Documentation/SubmittingPatches
>> @@ -63,10 +63,43 @@ t/README for guidance.
>> When adding a new feature, make sure that you have new tests to show
>> the feature triggers the new behaviour when it should, and to show the
>> feature does not trigger when it shouldn't.  Also make sure that the
>> +test suite passes after your commit.
> 
> This is not a new issue, but it sounds as if you do not have to test
> if you are not doing a new shiny toy.  Perhaps we should rephrase
> the last sentence a bit.
> 
> 	After any code change, make sure that the entire test suite
> 	passes.  After any documentation change, make sure that the
> 	resulting documentation set formats well.
Agreed.


> By the way, can you teach our Travis thing to check for the "make
> doc" failures?
Yes, expect a patch soonish.

> 
>> +We recommend to use our CI infrastructure to ensure your new feature
>> +passes all existing tests as well as your new tests on Linux, Mac, and
>> +(hopefully soon) Windows.  Follow these steps for the initial setup:
>> +
>> + (1) Sign in to GitHub: https://github.com
>> +     Please sign up first if you haven't already, it's free.
> 
> Three issues:
> 
> * None of the things utilized by the reader of this section looks
>   like "our" infrastructure to me.
OK. How about "We recommend to use the Travis CI infrastructure..."
instead?


> * The above makes it sound as if we recommend everybody to become
>   GitHub customer, which is not really the case.
Agreed. I assume your suggested wording below would be fine?


> * This is overly long and deserves to be its own separate section,
>   just like we have MUA specific hints, this is GitHub-Travis
>   specific hints.
Agreed.


> How about just saying
> 
> 	If you have an account at GitHub (and you can get one for
> 	free to work on open source projects), you can use their
> 	Travis CI integration to test your changes on Linux, Mac,
> 	and (hopefully soon) Windows.  See GitHub-Travis CI hints
> 	section for details.
> 	
> here, create a "GitHub-Travis CI hints" section just before "MUA
> specific hints" section, and move these numbered entries and the two
> paragraphs that follow to the new section.  As the introductory text
> for the new section itself, it may make sense to repeat a rephrased
> version of the above there, e.g.
> 
> 	--------------------------------------------------
>        GitHub-Travis CI hints
> 
> 	With an account at GitHub (you can get one for free to work
> 	on open source projects), you can use Travis CI to test your
> 	changes on Linux, Mac, and (hopefully soon) Windows.
> 
> 	Follow these steps for the initial setup:
> 
> 	(1) ...
Agreed. I also like Stefan's suggestion to move the CI stuff
into a separate file. Any objections to this?


> I'd mildly prefer to leave "Please sign up first" line out
> of the first entry.
OK, I will remove it. My intention was to express that you need a 
GitHub account to use Travis CI. 


Thanks for the review,
Lars


>> + ...
>> + (7) Enable Travis CI builds for your Git fork
>> +
>> +After the initial setup you can push your new feature branches to your
>> +Git fork on GitHub and check if they pass all tests here:
>> +https://travis-ci.org/<Your GitHub handle>/git/branches
>> +
>> +If they don't pass then they are marked "red". If that happens then
>> +click on the failing Travis CI job and scroll all the way down in the
>> +log. Find the line "<-- Click here to see detailed test output!" and
>> +click on the triangle next to the log line number to expand the detailed
>> +test output (example: https://travis-ci.org/git/git/jobs/122676187).
>> +Fix the problem and push an updated commit to your branch to ensure
>> +all tests pass.
>> +
>> +Do not forget to update the documentation to describe the updated
>> +behaviour of your new feature. It is currently a liberal mixture of US
>> and UK English norms for spelling and grammar, which is somewhat
>> unfortunate.  A huge patch that touches the files all over the place
>> only to correct the inconsistency is not welcome, though.  Potential

[PATCH v2] Documentation: add setup instructions for Travis CI

[larsxschneider]

From: Lars Schneider <larsxschneider@gmail.com>

Also change UK english "behaviour" to US english "behavior".

Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---

diff to v1:

* remind the reader to check documentation set formatting
* do not call Travis CI as "our infrastructure"
* do not make it sound as we want to make everyone a GitHub customer
* move detailed Travis CI instructions to a separate section
  (I thought about a separate file but I think there is not enough
  content at this point to justify it)

Thanks Junio, Stefan, and Matthieu for the review,
Lars


 Documentation/SubmittingPatches | 80 ++++++++++++++++++++++++++++++++---------
 1 file changed, 63 insertions(+), 17 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 98fc4cc..60e0e55 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -61,23 +61,28 @@ Make sure that you have tests for the bug you are fixing.  See
 t/README for guidance.

 When adding a new feature, make sure that you have new tests to show
-the feature triggers the new behaviour when it should, and to show the
-feature does not trigger when it shouldn't.  Also make sure that the
-test suite passes after your commit.  Do not forget to update the
-documentation to describe the updated behaviour.
-
-Speaking of the documentation, it is currently a liberal mixture of US
-and UK English norms for spelling and grammar, which is somewhat
-unfortunate.  A huge patch that touches the files all over the place
-only to correct the inconsistency is not welcome, though.  Potential
-clashes with other changes that can result from such a patch are not
-worth it.  We prefer to gradually reconcile the inconsistencies in
-favor of US English, with small and easily digestible patches, as a
-side effect of doing some other real work in the vicinity (e.g.
-rewriting a paragraph for clarity, while turning en_UK spelling to
-en_US).  Obvious typographical fixes are much more welcomed ("teh ->
-"the"), preferably submitted as independent patches separate from
-other documentation changes.
+the feature triggers the new behavior when it should, and to show the
+feature does not trigger when it shouldn't.  After any code change, make
+sure that the entire test suite passes.
+
+If you have an account at GitHub (and you can get one for free to work
+on open source projects), you can use their Travis CI integration to
+test your changes on Linux, Mac, and (hopefully soon) Windows.  See
+GitHub-Travis CI hints section for details.
+
+Do not forget to update the documentation to describe the updated
+behavior and make sure that the resulting documentation set formats
+well. It is currently a liberal mixture of US and UK English norms for
+spelling and grammar, which is somewhat unfortunate.  A huge patch that
+touches the files all over the place only to correct the inconsistency
+is not welcome, though.  Potential clashes with other changes that can
+result from such a patch are not worth it.  We prefer to gradually
+reconcile the inconsistencies in favor of US English, with small and
+easily digestible patches, as a side effect of doing some other real
+work in the vicinity (e.g. rewriting a paragraph for clarity, while
+turning en_UK spelling to en_US).  Obvious typographical fixes are much
+more welcomed ("teh -> "the"), preferably submitted as independent
+patches separate from other documentation changes.

 Oh, another thing.  We are picky about whitespaces.  Make sure your
 changes do not trigger errors with the sample pre-commit hook shipped
@@ -370,6 +375,47 @@ Know the status of your patch after submission
   entitled "What's cooking in git.git" and "What's in git.git" giving
   the status of various proposed changes.

+--------------------------------------------------
+GitHub-Travis CI hints
+
+With an account at GitHub (you can get one for free to work on open
+source projects), you can use Travis CI to test your changes on Linux,
+Mac, and (hopefully soon) Windows.  You can find a successful example
+test build here: https://travis-ci.org/git/git/builds/120473209
+
+Follow these steps for the initial setup:
+
+ (1) Fork https://github.com/git/git to your GitHub account.
+     You can find detailed instructions how to fork here:
+     https://help.github.com/articles/fork-a-repo/
+
+ (2) Open the Travis CI website: https://travis-ci.org
+
+ (3) Press the "Sign in with GitHub" button.
+
+ (4) Grant Travis CI permissions to access your GitHub account.
+     You can find more information about the required permissions here:
+     https://docs.travis-ci.com/user/github-oauth-scopes
+
+ (5) Open your Travis CI profile page: https://travis-ci.org/profile
+
+ (6) Enable Travis CI builds for your Git fork.
+
+After the initial setup, Travis CI will run whenever you push new changes
+to your fork of Git on GitHub.  You can monitor the test state of all your
+branches here: https://travis-ci.org/<Your GitHub handle>/git/branches
+
+If a branch did not pass all test cases then it is marked with a red
+cross.  In that case you can click on the failing Travis CI job and
+scroll all the way down in the log.  Find the line "<-- Click here to see
+detailed test output!" and click on the triangle next to the log line
+number to expand the detailed test output.  Here is such a failing
+example: https://travis-ci.org/git/git/jobs/122676187
+
+Fix the problem and push your fix to your Git fork.  This will trigger
+a new Travis CI build to ensure all tests pass.
+
+
 ------------------------------------------------
 MUA specific hints

--
2.5.1

Re: [PATCH v2] Documentation: add setup instructions for Travis CI

[Matthieu Moy]

larsxschneider@gmail.com writes:

> +on open source projects), you can use their Travis CI integration to
> +test your changes on Linux, Mac, and (hopefully soon) Windows.  See

Nit: I'd write Linux, Mac (and hopefully soon Windows). Doesn't deserve
a reroll IHMO.

Other than that, the patch looks good to me.

Thanks,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v2] Documentation: add setup instructions for Travis CI

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> larsxschneider@gmail.com writes:
>
>> +on open source projects), you can use their Travis CI integration to
>> +test your changes on Linux, Mac, and (hopefully soon) Windows.  See
>
> Nit: I'd write Linux, Mac (and hopefully soon Windows). Doesn't deserve
> a reroll IHMO.
>
> Other than that, the patch looks good to me.

Thanks; I can do two replacements in place ;-)

Re: [PATCH v2] Documentation: add setup instructions for Travis CI

[Lars Schneider]

On 02 May 2016, at 10:48, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:

> larsxschneider@gmail.com writes:
> 
>> +on open source projects), you can use their Travis CI integration to
>> +test your changes on Linux, Mac, and (hopefully soon) Windows.  See
> 
> Nit: I'd write Linux, Mac (and hopefully soon Windows). Doesn't deserve
> a reroll IHMO.
> 
> Other than that, the patch looks good to me.
This is fine with me. Thanks for the review!

- Lars