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 requirements.txt file for readthedocs. #8

Merged
merged 1 commit into from
Apr 4, 2018

Conversation

jguiditta
Copy link
Member

Hopefully this gets fixed soon, but currently readthedocs doesn't
support Pipfile, only requirements.txt. This patch freezes what we have
managed by pipenv currently, and should be updated as Pipfile/lock are
updated until readthedocs gets the support we need.

Signed-off-by: Jason Guiditta [email protected]

Copy link
Member

@fuzzball81 fuzzball81 left a comment

Choose a reason for hiding this comment

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

The requirements.txt file is a little ugly with all of the sha256 hashes. We might want to take those out to reduce future diff sizes.

@jguiditta
Copy link
Member Author

@fuzzball81 The thing is, this should be a temporary file (it is generated by pipenv based on our lock file, and is not really meant to be used by humans). I didn't want to add it at all, but for now, readthedocs doesn't seem to want to build without it. I saw they have an open issue for pipfile support[1], so I expect this file to be transient, and I will remove it the second I find readthedocs has updated their support. I agree that the SHAs are not ideal, but they are meant to be a security feature of pip (and thus pipenv). And I am a little concerned this still wont work on rtd, because we have some system requirements on pipenv install which they are not likely to have available. If this patch is merged, and the docs are still no good, I may have to look into an alternate way to get the docs over there anyway (we can compile them just fine, maybe I can just push them over already compiled - I'll look to see if that is an option).

[1] readthedocs/readthedocs.org#3181

@jguiditta
Copy link
Member Author

jguiditta commented Apr 4, 2018

OK, I did some poking, and no luck on just pushing compiled docs up ourselves. And on further reflection, we really only have 1 dep we need to pull in - koji. I will update this to make the req.txt just have that and see what happens. I think I will likely have to add a bit I saw on the rtd documentation to stub out the required C libs, so if the doc build fails, that will be the next round.

Hopefully this gets fixed soon, but currently readthedocs doesn't
support Pipfile, only requirements.txt. This should require few updates,
as this project is not intended to have a lot of other dependencies.

Signed-off-by: Jason Guiditta <[email protected]>
@jguiditta
Copy link
Member Author

OK, tests are good, if you are ok with current rev, let's get it in and see if it is enough to get the full docs. If not, I'll submit additional PRs to get there

@fuzzball81 fuzzball81 merged commit 3f8554e into release-depot:master Apr 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants