Skip to content
This repository was archived by the owner on Dec 12, 2018. It is now read-only.

Conversation

@rdegges
Copy link
Contributor

@rdegges rdegges commented Dec 21, 2016

This will help us avoid issues with docs search across SDKs in the future.

NOTE: Please don't merge this yet, please review.

This will help us avoid issues with docs search across SDKs in the future.
@mrioan mrioan self-requested a review December 21, 2016 19:02
@mrioan mrioan self-assigned this Dec 21, 2016
mrioan
mrioan previously requested changes Dec 21, 2016
Copy link
Contributor

@mrioan mrioan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @rdegges. Additional comment, please target 1.2.x or 1.3.x but not master (this comment is for current status of the repository, in the future master will be a target branch again). There is a chance that 1.3.x will be released sooner than 1.2.x. Up to you to decide which is the proper branch for this PR.
Thanks for taking care of this! Back to you

ci/install.sh Outdated

info "Installing Sphinx..."
pip -q install --user sphinx &> $WORKDIR/target/pip.log
pip -q install -r requirements.txt &> $WORKDIR/target/pip.log
Copy link
Contributor

@mrioan mrioan Dec 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this requirements file is only for docs, can you please move it from root and place it inside the docs dir? You could also put it inside ci (where the install.sh file exists).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asume --user sphinx is not needed for any reason, please confirm. I just want to be sure why it was removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct: --user sphinx is now deprecated with the new Travis stuff =)

@rdegges
Copy link
Contributor Author

rdegges commented Dec 21, 2016

@mrioan just updated the file location. Let me know if this works for ya!

@mraible mraible dismissed mrioan’s stale review January 5, 2017 00:44

Randall has satisfied Mario's requests

@mraible mraible merged commit e2561c8 into master Jan 5, 2017
@mraible mraible deleted the docs-update branch January 5, 2017 03:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants