-
Notifications
You must be signed in to change notification settings - Fork 215
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
Continuous Documentation #344
Conversation
Continuous documentation made on every push made to the repository. Sets up miniconda first, installs the necessary packages, builds the documentation pages and then deploys website to Zeit Now static site host.
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/gmt/pygmt/119cxm068 |
Ok, this is almost ready! Previews of the website can be viewed by clicking on the 'View deployment' button: The bot is a lot noisier than I expected, but I've toggled on a silent flag that should 🤞 stop it from commenting on Pull Requests and commits once this PR is merged in. Another thing worth considering is the related Deploy Summary Integration app (see review) that would detect changes to the pages and post a screenshot and direct link to the said changes. This will need to be installed (and might add more noise to our PRs) so looking to see what everyone thinks. |
@weiji14 thanks for taking the time! This looks really cool. Is the now service building the docs or are they being deployed from the CI? Since all our CIs build the docs anyway, would it be easier to deploy from there? The problem is that keep these build systems alive takes quite a bit of time and effort, so it would be good to explore all options before adding a new one. |
Yes, the now service is building them using
I hinted at #342 (comment) that this is indeed possible, but would require setting a token/secret variable. We could probably disable those documentation builds in a separate PR to speed them up? The thing is, each of the built documentation sites have to be hosted online somewhere and given a unique url, which none of our current CIs are able to provide. Github Pages does do a build but is limited to one url. Another option I've looked at is netlify that does CD as well, but their pricing structure isn't as good as Now's. |
Slightly off-topic but have you guys ever thought of migrating to gitlab? They have the possibility of ci/cd stuff directly built in so that you don't have to use different external apps that one has to somehow connect to... And the most important part: a single point of maintenance! Have a look here: https://docs.gitlab.com/ee/ci/introduction/index.html |
I’m not too keen on that since he docs builds are a sort of test. But to all builds need to do this (probably only one per OS would be good).
And using a key won’t work for forks because they aren’t decrypted for these builds (rightly so). But there are ways of storing build artefacts in Azure and Travis (which could be the html).
Yes but the problem is that everyone is here on GitHub, which makes it much easier to recruit contributors. But it is tempting. |
Agreed that the documentation builds (which creates figures) does provide provide some 'integration' testing. Also with the Gitlab argument, Github does have Github Actions now which can be used to do CI/CD, but again, this topic is getting sidetracked too much... Are there any outstanding issues with this PR @leouieda, or is it ready for approval/merge? I'm actually waiting to get this merged in to start working on some gallery examples. The workshop is coming up soon... |
@weiji14 would you mind inserting some comments in the two files included here about what they are? Also, did this setup require signing up for these services? If so, were they done with your personal account? Would you mind posting some more information about this here so we can I have a record in case someone else need to manage this in the future? One thing we want to avoid is having a single person be able to access/manage things (which reminds me, we need to add you to Azure and PyPI). |
Sorry for holding this up but PRs are meant for discussion before we commit to a given feature. It’s much easier to add things than it is to remove them later (granted, not in the case of this PR). I want to encourage a culture of deeper discussion of things before giving the 👍 and merging, even if that means going a bit slower in PyGMT than GMT core. |
No don't apologize! I totally understand from a maintenance point of view (having worked on >10 year old legacy code before). A quick search shows that it isn't easy to add comments json files (see https://stackoverflow.com/questions/244777/can-comments-be-used-in-json) but I'll look around a bit more. In terms of the Now account setup, I've set up a 'gmt' organization and I'm currently the 'owner', with @seisman being an |
@weiji14 i thought that might be the case. Don’t worry too much about it. What you can do is include a section in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 @weiji14 looks good to me!
As a side note: when you merge a PR, please edit the commit message. Remove the messages from irrelevant commits and make a condense it into a good descriptive message (and title) so we can easily track things in the history. See the commit message for this PR as an example. cc @seisman
Description of proposed changes
Making it easier to preview documentation changes in Pull Requests via 'Continuous Documentation'. Currently deployed to serverless infrastructure on Zeit Now.
Zeit Now is meant to be a zero-config/serverless, but that's for the javascript world. I've hacked the
package.json
to getsphinx
andpygmt
up and running to build the docs. The static files are then copied to apublic
folder where it gets deployed automatically to a url that looks likepygmt-abcdef.now.sh
. You can see an example at https://pygmt-l8nmyn8mx.now.sh/.Merge after #343, Fixes #342
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.