Closed mkoeppe closed 7 years ago
Oops, I overlooked that pull request. Sorry Matthias! I went through it, and it sounds very reasonable. Two questions and one obvious comment:
Other than that, any comment anyone, or should I just clic merge?
This adds some non trivial boilerplate. Do you foresee (in the long run) a way to factor out this boiler plate in e.g. sage-package?
Not sure about this. Perhaps .travis-deploy-doc.sh could become a script in sage-package at some point. But it is quite possible that users will have to edit this script to customize it.
Would we want the doc to be available at XXX/sage_sample/ rather than XXX/sage_sample/doc/html?
I would rather imagine that users have some hand-written HTML, such as a project home page, in XXX/sage_sample/, which would link to doc/html.
On Thu, May 04, 2017 at 10:50:02PM -0700, Matthias Köppe wrote:
Not sure about this. Perhaps .travis-deploy-doc.sh could become a script in sage-package at some point. But it is quite possible that users will have to edit this script to customize it.
Ok, thanks for the insight! Let's keep thinking in background about the best compromise between factoring out and duplicating.
I would rather imagine that users have some hand-written HTML, such as a project home page, in XXX/sage_sample/, which would link to doc/html.
That's a good point indeed. Presumably the two use cases will occur. The question is which default we want to choose. Arguably, one could consider that we want to make it as trivial as possible for authors of small packages -- for which using the first page of the doc as project page is simple and good enough -- while authors of larger package will know how to change the target directory.
Up to the comment I left in the diff, this is looking good, and I would be up pressing "merge". Comment anyone?
How about turning on Travis CI for this repository?
How about turning on Travis CI for this repository?
Oh right; I did not realize it was not already on :-) Please proceed!
Sadly people keep deleting old binary distributions from the Sage servers -- so it's not possible any more to test a user package against old distributions. 7.1–7.4 are gone now. I'm adjusting the list of builds...
I think the Travis CI will be activated if you make a single commit to one of the repository's branches now.
Trivial commit pushed on mater. We probably will want to add relevant badges in the README.
I still only see "This is not an active repository" at https://travis-ci.org/sagemath/sage_sample/ ...
It's active now :-)
And failing, but that's just because that it's still using the old Sage versions that you updated in that PR.
Allright, that looks good, let's move on. Thank you Matthias!
Now it would make sense to follow the instructions in README and set up the cryptographic key so that deployment works... This is the only thing that's missing for a passing Travis CI build ... https://travis-ci.org/sagemath/sage_sample/jobs/229296891
Done. Well at least I think. However .travis-deploy-doc.sh hangs asking for a passphrase, when I thought I set no passphrase when defining the key. Any clue? https://travis-ci.org/sagemath/sage_sample/jobs/229341998
The deploy key stuff is new to me. In the past, for deployment via Travis, I've used a GitHub token (encrypted in a 'secure' environment variable in the .travis.yml)
Added a script and instructions for this. Result can be seen here: https://mkoeppe.github.io/sage_sample/doc/html/