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

Gh pages #8

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Gh pages #8

wants to merge 8 commits into from

Conversation

asmodehn
Copy link

@asmodehn asmodehn commented Apr 18, 2020

A simple attempt to integrate sphinx doc with github pages.

adding requirements.txt and fixing sphinx build command

installing pipenv, via system pip3, into user venv.

also installing setuptools

also installing wheel for python3

specifically installing python 3.7
push:
branches: [ master ]
pull_request:
branches: [ master ]
Copy link

Choose a reason for hiding this comment

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

If I understand this correctly, these two lines should be removed before merging otherwise any PR to master would update the docs.

The way tokens work though I believe this is true only for PRs opened where the author has commit rights to the repo.

Copy link
Author

Choose a reason for hiding this comment

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

good point... I m just discovering github actions, but maybe we want 2 workflows:

  • one for pull request that just attempt a build (doctests and other can be added later on)
  • one for push that build and deploy

Copy link
Owner

Choose a reason for hiding this comment

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

Right now the way it works is that any change to master rebuilds the Alloy docs on readthedocs.

@hwayne
Copy link
Owner

hwayne commented Apr 21, 2020

My main concern here is with conf.py: https://github.com/hwayne/alloydocs/blob/master/conf.py#L187. The intent there is to hide todos for the deployed version but show it to people on their local versions. Since that currently hardcodes readthedocs, it would show todos on github actions. Does actions set a special environmental variable we can use?

(We'd also need to make sure that this merge wouldn't break the RTD documentation, as that's where we're hosting the provision docs. Merging the github actions building would be more to make sure we could incorporate it seamlessly into the rest of the docs when the time is right.)

@asmodehn
Copy link
Author

We can use a github defined variable like GITHUB_ACTIONS :
https://help.github.com/en/actions/configuring-and-managing-workflows/using-environment-variables
My fork deploys there : http://www.asmodehn.fr/alloydocs/

I also setup a build on RTD :
https://alloydocs-fork.readthedocs.io/en/latest/
Looks like it worked out of the box (didn't have to set anything...)

@asmodehn
Copy link
Author

My personal concern however is with the python virtual environment setup.
We want both RTD and github (and local dev) to have the same version of every dependency, to avoid strange/hard-to-debug issues later on.

My personal choice for all my projects currently is pipenv, and it looks like RTD has had some discussion about it : readthedocs/readthedocs.org#3181

However I don't see it used at all in the build : https://readthedocs.org/projects/alloydocs-fork/builds/10889123/
There is also that lying around : readthedocs/readthedocs.org#4783

I think we would still need a .readthedocs.yml (maybe with a requirements.txt) file to be able to configure that properly: https://docs.readthedocs.io/en/stable/config-file/v2.html
Maybe even in the current master, so we can track the changes to the RTD config that this PR brings along...

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.

None yet

3 participants