[MRG + 1] [doc skip] docs(contributorGuide): add commit message magic commands to contributor guide #8024

alexandercbooth · 2016-12-08T23:06:42Z

Addresses #8019 by adding commit message magic commands to the contributor guide.

…mands to avoid unnecessarily building docs in CI

dalmia · 2016-12-09T03:25:41Z

CONTRIBUTING.md

@@ -12,7 +12,7 @@ likelihood of your contribution being merged.**
 How to contribute
 -----------------

-The preferred workflow for contributing to scikit-learn is to fork the 


Good work removing the PEP8 errors 👍

dalmia · 2016-12-09T03:27:17Z

CONTRIBUTING.md

@@ -48,11 +48,13 @@ GitHub, clone, and develop on a branch. Steps:
   $ git push -u origin my-feature
   ```

+  *Note*: if your PR includes changes to files under the ``doc/`` or ``examples/`` folders, include in your commit message ``[doc build]``. Otherwise, include in your commit message ``[doc skip]``. This will allow the CI to avoid unnecessarily building the docs.


I am not sure this is the best place to add this change. I feel it's better you add this as another bullet in the Pull Request Checklist section, below the part where documentation is mentioned.

Everything else seems good. Just needs a few formatting corrections:

I think the Note may not be needed

*If your PR

include [doc build] in your commit message

Otherwise, include [doc skip] in your commit message

Good thinking, that fits in much more naturally down there. Thanks @dalmia!

…umentation and minor formatting corrections

dalmia · 2016-12-09T04:28:02Z

CONTRIBUTING.md

@@ -113,6 +113,11 @@ following rules before you submit a pull request:
   scale in dimensionality: n_features is expected to be lower than
   100".

+-  If your PR includes changes to files under the ``doc/`` or
+   ``examples/`` folders, include ``[doc build]`` in your commit message.
+   Otherwise, include ``[doc skip]`` in your commit message. This will 


Thanks for making the quick range. I just have one more suggestion.
'message. This will' --> 'message which will'.
Also, you need to add the change in the same manner in doc/developers/contributing.rst

Good catch, I didn't realize that there was a contributor guide there too!

…tor guide to include CI magic commands for commit messages

dalmia · 2016-12-09T04:42:40Z

Once the tests pass, I feel this looks good.

alexandercbooth · 2016-12-09T04:44:13Z

Thanks for your feedback, @dalmia!

dalmia · 2016-12-09T04:48:11Z

Sure, no problem :)

jnothman · 2016-12-10T10:47:12Z

CONTRIBUTING.md

@@ -113,6 +113,11 @@ following rules before you submit a pull request:
   scale in dimensionality: n_features is expected to be lower than
   100".

+-  If your PR includes changes to files under the ``doc/`` or


No, we use this logic automatically.

What we need to be saying is more along the lines of:

describing the roles of the different CIs we use: travis runs on various linux settings, appveyor on various windows settings, both do some coverage analysis, and CircleCI builds the documentation for viewing (for which a URL is given in contributing.rst)

noting that [ci skip] appearing in the latest commit message will stop these running

noting that while some heuristics are used for Circle to decide between running a full HTML documentation build, including plots (make html) and a quick one (make html-noplot), putting one of [doc build], [doc quick] and [doc skip] will override this logic with the build including example gallery plots, the build excluding example gallery plots, and no documentation build.

Thanks, @jnothman!

Sorry, I think I misunderstood the comments in #7923 about what the markers did. Is something like this more appropriate?

A Note about CI:

Travis is used for testing on Linux platforms

Appveyor is used for testing on Windows platforms

CircleCI is used to build the docs for viewing

Please note that if one of the following markers appear in the latest commit message, the following actions are taken.

Commit Message Marker Action Taken by CI

[ci skip] CI is skipped completely

[doc skip] Docs are not built

[doc quick] Docs built, but exclude example gallery plots

[doc build] Docs built, including example gallery plots

Also, is there a reason OSX isn't tested (with Travis, for example)?

I don't think there's a strong reason OS X isn't tested except that we've rarely had problems that behaved differently between OS X and linux. It's not like we're building a GUI or interfacing with other processes.

and yes, that looks good. do it!

That's a good point, I use it on both and haven't run into an issue specific to either.

Great, fixed up and submitted. Thanks for your help!

jnothman · 2016-12-12T02:47:54Z

CONTRIBUTING.md

@@ -113,6 +113,21 @@ following rules before you submit a pull request:
   scale in dimensionality: n_features is expected to be lower than
   100".

+A Note about CI:


I'm not sure this is the right place. It doesn't fit before "You can also check for common errors".

Please change the heading to "Continuous Integration (CI)" and introduce it with a sentence saying "We use a few tools to ensure the integrity of our code and documentation for each commit in pull requests and merged into the master branch."

I also think this is more detail than necessary in Contributing.md and would happily see it only in the HTML guide.

OK, just to clarify, the suggestion here is to:

revert CONTRIBUTING.md to it's original state

change the title of the CI part as you suggest above in doc/developers/contributing.rst and also move it in that document to a more appropriate place

As for where to move it, perhaps before or after the "Testing and improving test coverage" part?

Thanks!

amueller

Argh my reviews were pending...

amueller · 2016-12-09T17:42:01Z

CONTRIBUTING.md

@@ -113,6 +113,11 @@ following rules before you submit a pull request:
   scale in dimensionality: n_features is expected to be lower than
   100".

+-  If your PR includes changes to files under the ``doc/`` or
+   ``examples/`` folders, include ``[doc build]`` in your commit message.


This will actually automatically detected. These are just overwrites.

I'm sorry about that, @amueller

Here are the latest proposed changes

Leaving CONTRIBUTING.md as it was originally and adding this to contributing.rst:

Continuous Integration (CI)

Travis is used for testing on Linux platforms

Appveyor is used for testing on Windows platforms

CircleCI is used to build the docs for viewing

Please note that if one of the following markers appear in the latest commit message, the following actions are taken.

Commit Message Marker Action Taken by CI

[ci skip] CI is skipped completely

[doc skip] Docs are not built

[doc quick] Docs built, but exclude example gallery plots

[doc build] Docs built, including example gallery plots

My question is where is the most appropriate place in contributing.rst in which to insert this. My thoughts were perhaps below or above the "Testing and improving test coverage" part.

Thanks!

jnothman · 2016-12-13T00:54:30Z

Perhaps a `.. topic` at/near the bottom of Contributing Pull Requests is appropriate.

…

On 13 December 2016 at 04:50, alexandercbooth ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In CONTRIBUTING.md <#8024>: > @@ -113,6 +113,11 @@ following rules before you submit a pull request: scale in dimensionality: n_features is expected to be lower than 100". +- If your PR includes changes to files under the ``doc/`` or + ``examples/`` folders, include ``[doc build]`` in your commit message. I'm sorry about that, @amueller <https://github.com/amueller> Here are the latest proposed changes Leaving CONTRIBUTING.md as it was originally and adding this to contributing.rst: Continuous Integration (CI) - Travis is used for testing on Linux platforms - Appveyor is used for testing on Windows platforms - CircleCI is used to build the docs for viewing Please note that if one of the following markers appear in the latest commit message, the following actions are taken. Commit Message Marker Action Taken by CI [ci skip] CI is skipped completely [doc skip] Docs are not built [doc quick] Docs built, but exclude example gallery plots [doc build] Docs built, including example gallery plots My question is where is the most appropriate place in contributing.rst in which to insert this. My thoughts were perhaps below or above the "Testing and improving test coverage" part. Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8024>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEz60BMAntoo_7cnSm-6CaGcioqGoSPks5rHYlpgaJpZM4LIY4_> .

alexandercbooth · 2016-12-15T01:58:08Z

Thanks for the feedback, here's a current mockup, any thoughts?

jnothman · 2016-12-15T03:52:46Z

Move it to the end of the section with the other notes, and I think it's great! Now we just need to get Circle running again :\

…ests section

alexandercbooth · 2016-12-15T05:39:58Z

Great! Really appreciate all your help on this!

jnothman · 2016-12-15T05:51:57Z

doc/developers/contributing.rst

+   * Appveyor is used for testing on Windows platforms
+   * CircleCI is used to build the docs for viewing
+
+   Please note that if one of the following markers appear in the latest commit message, the following actions are taken.


Please wrap this line at 80 chars

did it myself (thanks to recent github feature that lets me do so...)

jnothman · 2016-12-15T06:01:54Z

Might as well wait for build and rendering at http://scikit-learn.org/circle?7617/_changed.html

jnothman · 2016-12-15T06:34:31Z

Build failed for silly reasons (will post issue). Merging.

jnothman · 2016-12-15T06:40:54Z

thanks @alexandercbooth!

alexandercbooth · 2016-12-15T13:26:25Z

Thanks so much or all your help, @jnothman!

[MRG] [doc skip] docs(contributorGuide): add commit message magic com…

ef977c9

…mands to avoid unnecessarily building docs in CI

dalmia suggested changes Dec 9, 2016

View reviewed changes

chore(contributorGuide): move CI magic commands to location after doc…

90e2e9e

…umentation and minor formatting corrections

dalmia suggested changes Dec 9, 2016

View reviewed changes

alexandercbooth added 2 commits December 8, 2016 23:33

[doc skip] chore(contributorGuide): minor formatting change on wording

c855722

[doc skip] chore(devContributing): echo changes in top level contribu…

652f7f5

…tor guide to include CI magic commands for commit messages

dalmia approved these changes Dec 9, 2016

View reviewed changes

raghavrv added the Documentation label Dec 9, 2016

raghavrv changed the title ~~[MRG] [doc skip] docs(contributorGuide): add commit message magic commands to contributor guide~~ [MRG + 1] [doc skip] docs(contributorGuide): add commit message magic commands to contributor guide Dec 9, 2016

raghavrv approved these changes Dec 9, 2016

View reviewed changes

jnothman requested changes Dec 10, 2016

View reviewed changes

alexandercbooth added 2 commits December 11, 2016 15:47

chore(readme): update to include formatting changes requested

b2d7fb5

chore(contributorRst): update to include formatting changes requested

87e77ea

jnothman mentioned this pull request Dec 11, 2016

document CI magic commit messages #8019

Closed

jnothman reviewed Dec 12, 2016

View reviewed changes

amueller reviewed Dec 12, 2016

View reviewed changes

alexandercbooth added 2 commits December 14, 2016 21:35

chore(CI): move CI/commit message magics to end of notes in Pull Requ…

6b50b8c

…ests section

chore(CI): remove CI/commit message magics

7b94ea0

jnothman reviewed Dec 15, 2016

View reviewed changes

jnothman approved these changes Dec 15, 2016

View reviewed changes

Fixed line wrapping

cc3121d

jnothman merged commit 9ec1a4c into scikit-learn:master Dec 15, 2016

sergeyf pushed a commit to sergeyf/scikit-learn that referenced this pull request Feb 28, 2017

DOC add CI details and commands to contributor guide (scikit-learn#8024)

5b9010a

Przemo10 mentioned this pull request Mar 17, 2017

update fork (#1) #8606

Closed

Sundrique pushed a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017

DOC add CI details and commands to contributor guide (scikit-learn#8024)

0727716

NelleV pushed a commit to NelleV/scikit-learn that referenced this pull request Aug 11, 2017

DOC add CI details and commands to contributor guide (scikit-learn#8024)

53a09f1

paulha pushed a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017

DOC add CI details and commands to contributor guide (scikit-learn#8024)

1d32673

maskani-moh pushed a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017

DOC add CI details and commands to contributor guide (scikit-learn#8024)

617acdb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG + 1] [doc skip] docs(contributorGuide): add commit message magic commands to contributor guide #8024

[MRG + 1] [doc skip] docs(contributorGuide): add commit message magic commands to contributor guide #8024

alexandercbooth commented Dec 8, 2016

dalmia Dec 9, 2016

alexandercbooth Dec 9, 2016

dalmia Dec 9, 2016

dalmia Dec 9, 2016

alexandercbooth Dec 9, 2016

dalmia Dec 9, 2016

alexandercbooth Dec 9, 2016

dalmia commented Dec 9, 2016

alexandercbooth commented Dec 9, 2016

dalmia commented Dec 9, 2016

jnothman Dec 10, 2016

alexandercbooth Dec 10, 2016

alexandercbooth Dec 10, 2016

jnothman Dec 11, 2016

jnothman Dec 11, 2016

alexandercbooth Dec 11, 2016

jnothman Dec 12, 2016

alexandercbooth Dec 12, 2016

amueller left a comment

amueller Dec 9, 2016

alexandercbooth Dec 12, 2016

jnothman commented Dec 13, 2016 via email

alexandercbooth commented Dec 15, 2016

jnothman commented Dec 15, 2016

alexandercbooth commented Dec 15, 2016

jnothman Dec 15, 2016

jnothman Dec 15, 2016

jnothman commented Dec 15, 2016

jnothman commented Dec 15, 2016

jnothman commented Dec 15, 2016

alexandercbooth commented Dec 15, 2016

Commit Message Marker	Action Taken by CI
[ci skip]	CI is skipped completely
[doc skip]	Docs are not built
[doc quick]	Docs built, but exclude example gallery plots
[doc build]	Docs built, including example gallery plots

[MRG + 1] [doc skip] docs(contributorGuide): add commit message magic commands to contributor guide #8024

[MRG + 1] [doc skip] docs(contributorGuide): add commit message magic commands to contributor guide #8024

Conversation

alexandercbooth commented Dec 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dalmia commented Dec 9, 2016

alexandercbooth commented Dec 9, 2016

dalmia commented Dec 9, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amueller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnothman commented Dec 13, 2016 via email

alexandercbooth commented Dec 15, 2016

jnothman commented Dec 15, 2016

alexandercbooth commented Dec 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnothman commented Dec 15, 2016

jnothman commented Dec 15, 2016

jnothman commented Dec 15, 2016

alexandercbooth commented Dec 15, 2016