-
Notifications
You must be signed in to change notification settings - Fork 7
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
WIP: Use upstream sphinx templates #21
Conversation
One part is not backwards compatible: variable |
Note this depends on moremoban/moban#26 |
templates/conf.py.jj2
Outdated
{%block SPHINX_EXTENSIONS%} | ||
{%endblock%} | ||
] | ||
{%block custom_varaibles%} |
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.
this is #20
templates/conf.py.jj2
Outdated
] | ||
{%block custom_varaibles%} | ||
{%endblock%} | ||
intersphinx_mapping = { |
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.
this is the only difference between the two conf.py.jj2 , and it doesnt seem worthwhile having two different templates if this is the only difference.
@@ -0,0 +1,21 @@ | |||
.. {{ project }} documentation master file, created by | |||
sphinx-quickstart on {{ now }}. |
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.
The "created by sphinx-quickstart" is a bit annoying, and is an example of how copy
targets might be modified , here after copy, replace "sphinx-quickstart" with moban
or yehua
{{ now }}
doesnt exist yet, and is a bad idea for re-generated files.
maybe we can delete these lines by building a slightly different 'include' command.
# The theme to use for HTML and HTML Help pages. See the documentation for | ||
# a list of builtin themes. | ||
# | ||
html_theme = 'alabaster' |
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.
html_theme
overrides previous html_theme
variable.
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.
The include is first. Then our template overrides the hardcoded value.
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.
got you
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.
long term ... I'll propose enhancements to sphinx-quickstart so these hacks can be removed.
.travis.yml
Outdated
- git clone https://github.com/sphinx-doc/sphinx ../sphinx | ||
- (cd .. && ln -s sphinx/sphinx/templates/quickstart/ sphinx-quickstart-templates) | ||
# Check upstream mobans are in sync | ||
- moban -m upstream.yml |
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.
I think this command will update sphinx related files in the repo and then the tests will be performed on top of them. Do you imagine that we update them offline? for example, when the build fails, and we try to update those sphinx files, e.g. Make.bat.jj2. How would the update work?
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.
Yea, build breaks due to upstream changes and we copy if the changes are good, or we pin the git clone
to last good sha if the changes are undesirable, or we break the sync if upstream isnt usable anymore.
Im doing the same with AppVeyor https://gitlab.com/coala/mobans/merge_requests/22 , and will bring those templates here when ive finished. Im going to try getting the appveyor.yml.jj2 merged upstream.
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.
I believe make.bat should not be templated and must be autogenerated. Refer this. I believe changes must be done in setup.py
to autogenerate this.
@@ -0,0 +1,209 @@ | |||
# -*- coding: utf-8 -*- |
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.
This appears twice in the file.
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.
.travis.yml
Outdated
- git clone https://github.com/sphinx-doc/sphinx ../sphinx | ||
- (cd .. && ln -s sphinx/sphinx/templates/quickstart/ sphinx-quickstart-templates) | ||
# Check upstream mobans are in sync | ||
- moban -m upstream.yml |
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.
Yea, build breaks due to upstream changes and we copy if the changes are good, or we pin the git clone
to last good sha if the changes are undesirable, or we break the sync if upstream isnt usable anymore.
Im doing the same with AppVeyor https://gitlab.com/coala/mobans/merge_requests/22 , and will bring those templates here when ive finished. Im going to try getting the appveyor.yml.jj2 merged upstream.
# The theme to use for HTML and HTML Help pages. See the documentation for | ||
# a list of builtin themes. | ||
# | ||
html_theme = 'alabaster' |
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.
long term ... I'll propose enhancements to sphinx-quickstart so these hacks can be removed.
4563612
to
dfd4b86
Compare
44e2091
to
a4be270
Compare
@@ -0,0 +1,13 @@ | |||
{% macro underlines(name) %} |
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.
I think it might be possible to share this macro with templates/docs/source/index.rst.jj2
@@ -0,0 +1,13 @@ | |||
{% macro underlines(name) %} | |||
{%- for i in range(name|length) -%}={%- endfor -%} |
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.
this algorithm doesnt account for wide-width glyphs. For that, I think jinja doesnt have built in functions to determine the width, so we need moban plugins.
But this is good enough for now.
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.
have you tried https://github.com/moremoban/moban#split_length?
Before merging, I want to sync a repo with these changes to check what it looks like. |
@@ -0,0 +1,6 @@ | |||
{% macro underlines(name) %} |
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.
what's the purpose of two index.rst.jj2 here?
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.
This one is for the source, and the other is for the user docs.
c.f. https://github.com/moremoban/pypi-mobans/pull/21/files#r194086916
No description provided.