Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add help for building the devguide documentation #206

Merged
merged 3 commits into from May 25, 2017
Merged

Add help for building the devguide documentation #206

merged 3 commits into from May 25, 2017

Conversation

Copy link
Contributor

@cjrh cjrh commented May 22, 2017

No description provided.

@the-knights-who-say-ni
Copy link

@the-knights-who-say-ni the-knights-who-say-ni commented May 22, 2017

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

@matrixise
Copy link
Member

@matrixise matrixise commented May 22, 2017

Maybe we could add a requirements.txt file with your PR. @zware ?

@matrixise
Copy link
Member

@matrixise matrixise commented May 22, 2017

Don't answer to my comment about the requirements.txt file, I see it in your PR.

Copy link
Member

@Mariatta Mariatta left a comment

Thanks @cjrh
We have something written in http://cpython-devguide.readthedocs.io/docquality.html#helping-with-the-developers-guide
Instead of adding a new page, maybe try expanding that page.

BUILDING.rst Outdated
(env) $ pip install -r requirements.txt
(env) $ make html
After the processing completes, the new HTML documents will be available in the ``_build/html``
Copy link
Member

@Mariatta Mariatta May 22, 2017

Choose a reason for hiding this comment

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

I would say "after the build is completed"

Copy link
Contributor Author

@cjrh cjrh May 22, 2017

Choose a reason for hiding this comment

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

Thanks! I'll move these notes to the page you mentioned, and change the wording.

Copy link
Collaborator

@willingc willingc May 22, 2017

Choose a reason for hiding this comment

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

Thanks @cjrh. I like the more explicit directions that you have for working with the devguide code. It will be a nice addition to the page Mariatta references. 👍

@cjrh
Copy link
Contributor Author

@cjrh cjrh commented May 22, 2017

@Mariatta I've moved the instructions for virtualenv and using requirements.txt into docquality.rst.

@zware
Copy link
Member

@zware zware commented May 22, 2017

To mirror cpython, we could have a make venv target to take care of actually making the venv (as a separate exercise, you could go so far as to add targets and dependencies such that make html creates the venv for you and uses it, if it doesn't already exist. Both here and in cpython).

@cjrh
Copy link
Contributor Author

@cjrh cjrh commented May 22, 2017

@zware Good idea! I've found the venv entry in the cpython/Doc Makefile, and I'm learning how to set up dependencies inside makefiles.

@cjrh
Copy link
Contributor Author

@cjrh cjrh commented May 22, 2017

@zware @Mariatta I've now set up the Makefile to automatically create and use a virtualenv for all the targets requiring sphinx-build. The docs have now gone back to simply $ make html, but I point out that the builder is going to create a virtualenv behind the scenes.

Copy link
Member

@zware zware left a comment

LGTM. Could wrap the changed paragraph in docquality.rst to 80 columns, but that's not a big deal.

@Mariatta
Copy link
Sponsor Member

@Mariatta Mariatta commented May 22, 2017

As part of local testing this PR, I created a new virtual environment called venv_pr206, and activated that environment. When I ran pip install .., the dependencies were installed in venv_pr206.
I'm getting an error when trying to do make html because it is now looking for venv.
So maybe we still need to document that people should be using the environment name venv locally?

@cjrh
Copy link
Contributor Author

@cjrh cjrh commented May 22, 2017

@Mariatta I think the idea from @zware suggestion was that people no longer need to create a venv at all: simply $ make html does it all automatically. I'll go check to see if the docs still say that a user must create a venv...

@cjrh
Copy link
Contributor Author

@cjrh cjrh commented May 22, 2017

Ok, I checked that section again, it doesn't mention that a venv needs to be created. So the user can ignore that a venv is even being used, because from now on $ make html will do everything automatically.

@zware I also wrapped to 80.

@zware
Copy link
Member

@zware zware commented May 25, 2017

@Mariatta What was the error you got? Was it due to already having a venv activated?

@Mariatta
Copy link
Sponsor Member

@Mariatta Mariatta commented May 25, 2017

I think it was because I have an existing virtual environment called venv prior to this.
Is this suppose to update my existing venv with the new requirements?

@zware
Copy link
Member

@zware zware commented May 25, 2017

Hmm, that shouldn't be; can you reproduce it? As is, it should probably just use any venv named venv without modifying it at all. We could make the venv target depend on requirements.txt to try to keep things up to date.

@Mariatta
Copy link
Sponsor Member

@Mariatta Mariatta commented May 25, 2017

Ok I cleaned up my venv and tried this again.
Everything works :)

@Mariatta Mariatta merged commit a5ff058 into python:master May 25, 2017
1 check passed
@Mariatta
Copy link
Sponsor Member

@Mariatta Mariatta commented May 25, 2017

Thanks everyone!
Congrats @cjrh on your first contribution to the Dev Guide 🎉

@cjrh cjrh deleted the docbuildhelp branch May 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants