[MRG+1] DOC adding info about circleci build artifacts #7855

dalmia · 2016-11-11T19:03:31Z

Reference Issue

What does this implement/fix? Explain your changes.

Added the information about accessing the circleci build artifacts when making doc changes.

Any other comments?

Still haven't included as to how the artifacts should be accessed. Should the formula mentioned in the thread be included? If not, then we need some way of letting them know how to access it or else adding this won't make much of a difference.

dalmia · 2016-11-12T19:02:45Z

@nelson-liu Were you suggesting something like this?

nelson-liu · 2016-11-12T19:37:34Z

thanks for your contribution, I think it would be good to provide info about how to access the built artifacts (with the formula). Make sure you clarify the distinction between PR # and Build # as well, since Build # is the one that matters in this case.

dalmia · 2016-11-13T08:05:41Z

Thanks for the feedback. Will get back with the changes.
I did mention creating a PR and updating an existing one. Please let me know if this needs to be clarified more.

dalmia · 2016-11-14T18:47:10Z

@nelson-liu Could you please take a review of my changes? Would be really helpful. Thanks.

nelson-liu · 2016-11-14T20:27:03Z

doc/developers/contributing.rst

@@ -383,6 +383,13 @@ documentation with the maths makes it more friendly towards
 users that are just interested in what the feature will do, as
 opposed to how it works "under the hood".

+You may also be asked to show your changes when it's built. When you create
+a pull request or make changes in an existing one, circleci automatically
+builds the docs. Thus, you can easily view your changes in the built


hmm, I don't think circleci builds the docs if there are no changes to them?

nelson-liu · 2016-11-14T20:27:29Z

doc/developers/contributing.rst

+builds the docs. Thus, you can easily view your changes in the built
+artifacts using the following formula:
+
+``https://{BUILD_NUMBER}-843222-gh.circle-artifacts.com/0//home/ubuntu/scikit-learn/doc/_build/html/stable/index.html``


could you explain how to find the build number and note that it isn't the same as the pr # on github?

nelson-liu · 2016-11-14T20:27:54Z

thanks for the ping @dalmia, sorry for not following up on this.

dalmia · 2016-11-14T20:58:53Z

No problem @nelson-liu, will make the desired changes and follow up..

dalmia · 2016-11-15T23:21:55Z

@nelson-liu Please let me know if there are any more changes to be made.

jnothman · 2016-11-16T03:19:40Z

doc/developers/contributing.rst

+automatically builds them. Thus, you can easily view your changes in the built
+artifacts using the following formula:
+
+``https://{BUILD_NUMBER}-843222-gh.circle-artifacts.com/0//home/ubuntu/scikit-learn/doc/_build/html/stable/index.html``


With scikit-learn/scikit-learn.github.io#5, we could shorten this URL to http://scikit-learn.org/circle?{BUILD_NUMBER}

I was really hoping we didn't have to put that long a URL in the docs. This makes it so much cleaner. Thank you for letting me know. I'll make the change.

jnothman · 2016-11-16T04:20:15Z

doc/developers/contributing.rst

@@ -383,6 +383,17 @@ documentation with the maths makes it more friendly towards
 users that are just interested in what the feature will do, as
 opposed to how it works "under the hood".

+You may also be asked to show your changes when it's built. When you create
+a pull request or make changes in an existing one modifying the docs, circleci


circleci -> CircleCI

jnothman · 2016-11-16T04:20:36Z

doc/developers/contributing.rst

+automatically builds them. Thus, you can easily view your changes in the built
+artifacts using the following formula:
+
+``http://scikit-learn.org/circle?{BUILD_NUMBER}``


(Btw, this doesn't work yet. Awaiting review.)

jnothman · 2016-11-16T04:20:49Z

doc/developers/contributing.rst

+
+``http://scikit-learn.org/circle?{BUILD_NUMBER}``
+
+Note: When you visit the details page of the circleci tests, you can find your


dalmia · 2016-11-16T04:46:38Z

Waiting for the PR to be merged then!

jnothman · 2016-11-16T05:02:20Z

LGTM pending scikit-learn/scikit-learn.github.io#5

nelson-liu

scikit-learn/scikit-learn.github.io#5 is really neat btw, thanks @jnothman

nelson-liu · 2016-11-16T07:57:21Z

doc/developers/contributing.rst

+
+Note: When you visit the details page of the CircleCI tests, you can find your
+BUILD_NUMBER mentioned as 'build #' which is different from your pull request number
+present as 'pull/#'.


your pull request number present as -> your pull request number, which is presented as...

i found pull/# quite confusing for some reason, where did you get the info from? Perhaps clarify that it's the PR number on github

@nelson-liu Please let me know if the reference I provided clears it up or should I be making any changes. Thanks.

yes, it's good enough. can you fix the typo/grammar, though?

nelson-liu · 2016-11-16T08:01:34Z

doc/developers/contributing.rst

+``http://scikit-learn.org/circle?{BUILD_NUMBER}``
+
+Note: When you visit the details page of the CircleCI tests, you can find your
+BUILD_NUMBER mentioned as 'build #' which is different from your pull request number


i can't find where on the CI results page it says 'build #'...perhaps try to clarify this? maybe mention that it's in the circleci URL itself? that's a bit clunky, though...

I was using this as my reference for both PR as well as Build #. Thought this has both of them clearly mentioned.

Do you feel I should be changing it then? Because mentioning both of them makes it a bit messy, I suppose.

whoops, it looks different on my screen

I'd prefer the info to be something that would hopefully not change soon? I feel like the URL setup is a lot more static (in the sense that one day they can simply decide to radically change the UI), but this is fine as well.

You are certainly correct there and I am also confused as to why we have different screens. Just that searching in the URL seems a bit clumsy, and this one is a lot cleaner. As you say, let's stick with this for now and reiterate if needed in the future.

nelson-liu

seems good to me now, thanks @dalmia

dalmia · 2016-11-18T06:55:34Z

Thanks for the thorough review @nelson-liu

jnothman · 2016-11-20T02:16:23Z

Happy with this, but I suspect we should fix #7815 first.

dalmia · 2016-11-20T08:06:15Z

Please give me a week, busy with exams, and then I'll start working on that issue if it's still open.

dalmia · 2016-11-29T12:44:11Z

@jnothman Since #7815 is fixed now, is this ready for merge?

jnothman · 2016-11-29T12:52:34Z

Yes, I think I can merge this without another +1.

jnothman · 2016-11-29T12:52:55Z

Thanks @dalmia!

…7855)

DOC adding info about circleci build artifacts

7e563e8

DOC adding formula for viewing built artifacts

bd6cb30

nelson-liu suggested changes Nov 14, 2016

View reviewed changes

DOC adding note on finding build #

6cc861d

jnothman mentioned this pull request Nov 16, 2016

Add page to provide CircleCI artifact URL shortener scikit-learn/scikit-learn.github.io#5

Merged

jnothman reviewed Nov 16, 2016

View reviewed changes

DOC adding the shorter URL

2b1afa7

jnothman requested changes Nov 16, 2016

View reviewed changes

DOC minor updates

50f04a9

jnothman changed the title ~~[WIP] DOC adding info about circleci build artifacts~~ [MRG+1] DOC adding info about circleci build artifacts Nov 16, 2016

nelson-liu suggested changes Nov 16, 2016

View reviewed changes

DOC: fixing typo

8a26a80

nelson-liu approved these changes Nov 18, 2016

View reviewed changes

jnothman approved these changes Nov 20, 2016

View reviewed changes

jnothman merged commit 6cbef3e into scikit-learn:master Nov 29, 2016

dalmia deleted the 7451 branch November 29, 2016 20:40

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

[MRG+1] DOC adding info about circleci build artifacts (scikit-learn#…

e48a48d

…7855)

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

[MRG+1] DOC adding info about circleci build artifacts (scikit-learn#…

49e3194

…7855)

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

[MRG+1] DOC adding info about circleci build artifacts (scikit-learn#…

516218b

…7855)

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

[MRG+1] DOC adding info about circleci build artifacts (scikit-learn#…

307934e

…7855)

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

[MRG+1] DOC adding info about circleci build artifacts (scikit-learn#…

a1b8f90

…7855)


		``http://scikit-learn.org/circle?{BUILD_NUMBER}``

		Note: When you visit the details page of the circleci tests, you can find your

Uh oh!

[MRG+1] DOC adding info about circleci build artifacts #7855

[MRG+1] DOC adding info about circleci build artifacts #7855

Uh oh!

Conversation

dalmia commented Nov 11, 2016

Reference Issue

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

dalmia commented Nov 12, 2016

Uh oh!

nelson-liu commented Nov 12, 2016

Uh oh!

dalmia commented Nov 13, 2016

Uh oh!

dalmia commented Nov 14, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nelson-liu commented Nov 14, 2016

Uh oh!

dalmia commented Nov 14, 2016

Uh oh!

dalmia commented Nov 15, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dalmia commented Nov 16, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnothman commented Nov 16, 2016

Uh oh!

nelson-liu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nelson-liu Nov 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nelson-liu left a comment

Choose a reason for hiding this comment

Uh oh!

dalmia commented Nov 18, 2016

Uh oh!

jnothman commented Nov 20, 2016

Uh oh!

dalmia commented Nov 20, 2016

Uh oh!

dalmia commented Nov 29, 2016

Uh oh!

jnothman commented Nov 29, 2016

Uh oh!

jnothman commented Nov 29, 2016

Uh oh!

Uh oh!

dalmia commented Nov 16, 2016 •

edited

Loading

nelson-liu Nov 17, 2016 •

edited

Loading