sphinx-bootstrap-theme icon indicating copy to clipboard operation
sphinx-bootstrap-theme copied to clipboard

html_theme_path docs out of sync

Open svenevs opened this issue 5 years ago • 4 comments

As of #173 doing

html_theme_path = sphinx_bootstrap_theme.get_html_theme_path()

is not necessary (since you also added the entry points to setup.py). Removing that in the demo's conf.py though, since it's not actually using a proper install (just inserting path), you'll get a build failure.

I started playing with the Makefile, just gonna include a patch view and then explain

diff --git a/Makefile b/Makefile
index 193d50b..b8af352 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,7 @@
 # Makefile: For Python3 development.
 # Because fabric doesn't work in Py3 :(

-.PHONY: clean demo demo_server
+.PHONY: clean demo demo_server dist env realclean

 clean:
        rm -rf "dist" \
@@ -9,8 +9,21 @@ clean:
                "demo/build" \
                "sphinx_bootstrap_theme.egg-info" \

-demo:
-       cd demo && make html
+realclean: clean
+       rm -rf venv
+
+dist:
+       python3 setup.py sdist
+       python3 setup.py bdist_wheel --universal
+
+# make an editable install for development
+env:
+       virtualenv venv
+       source venv/bin/activate && pip install -r requirements.txt
+       source venv/bin/activate && pip install -e .
+
+demo: dist env
+       source venv/bin/activate && (cd demo && make html)

 # PORT allows you to specify a different port if say
 # port 8000 is currently in use

Pretty dirty, but the idea is you need to legitimately install this in order to actually make use of the entry point.

What exactly is requirements.txt here for? Is it related to this fabfile.py thing?

I was going to update the docs, but if we can't lead by example in our own conf.py, well shame on us lol. So I just threw it in a virtual environment for shiggles, and it worked fine.

  1. Can we just omit html_theme_path documentation? Users should not need it anymore, better to remove so we don't get confusion about whether it's a list or string.
  2. Should we go as far as to raise an exception in that method indicating it is no longer used / deprecated?
  3. For "lead-by-example", even though we don't have any unit tests, can we setup tox? It's really quite wonderful, it works on top of virtualenv, so you could do say tox -e flake8 to run lint checks or tox -e demo to build the demo (and probably server too, though I'd have to double check).
    • Is the requirements.txt for this theme or the fab stuff?
    • I can also add a tox -e dist that will create the dist/ folder for uploading files.

Let me know what you want to do here (even if it's just remove outdated docs pr and nothing else xD).

svenevs avatar Aug 26 '18 12:08 svenevs

@svenevs --

  • Overall changes look like a good direction!
  • requirements.txt is just to help development of the theme. Not functionally needed for users of the theme. Feel free to remove if we have some better way to get deps consistently installed for our development
  • Fabfile isn't necessary, and we can definitely do a "make only" workflow. On some other projects, I have been using invoke and liking that. But I'm cool with whatever... If tox is a good fabric / make replacement for all workflows, I'd love to see it and learn it!
  • Fine to omit html_theme_path everywhere. I'd only raise an exception if using it is now actually going to break things. But not that big a deal. We can just do a major semver release and take the support hit.
  • I'm fine with unittests, but we actually don't have much python in this thing. If it's useful, go for it! If not, we're likely looking at things like functional tests of a webpage with JS executing (which I do have a lot of familiarity with as that's been my day job for years now), but not sure the value it will add versus the work it will take.

In summary, use your best judgment and we'll probably just do that. I'm days-ish away from the birth of kiddo number two, so may disappear into the ether of newborn care soon. If I'm not around and you're confident with changes, just go ahead and release after your PR!

(Also, do we have you set up with publish rights on pypi? If not, ping me and let's get that set up ASAP)

Thanks!!!

ryan-roemer avatar Aug 27 '18 04:08 ryan-roemer

Awwwwwww so cool congratulations!

Ok I'll throw something together in a PR hopefully this weekend, there won't actually be a need to release on PyPi, it's just docs and tooling (your right, there's no reason to raise an exception).

The idea behind tox is you don't rely on anything related to platform specific utilities like make or our currently hard-coded python3, it uses the python that you installed tox for. So even Windows users run the same commands.

The real question I guess is will it enable potential frontend stuff like running the server or validating html or linting js. I'll ask around and see what I can come up with and include more info in the PR :)

How is the demo website actually built via GH pages though? That may be problematic for the "lead by example" in the demo itself, but as long as the docs are clear it'll be fine either way.

svenevs avatar Aug 28 '18 16:08 svenevs

All sounds good.

The GitHub pages thing is a manual, simple process:

  1. In master run make demo. Stash build/html files
  2. Checkout out gh-pages branch (or have a separate repo clone already checked out there).
  3. Nuke all non-dotfiles and copy in new build/html files at root directory level.

I'm fine with automating this in the future or whatever (even via CI too on version changes)...

ryan-roemer avatar Aug 31 '18 22:08 ryan-roemer

Oh. Very interesting. I never realized it comes from the gh-pages branch on this repository.

Hmm. Very intriguing. This can definitely be automated.

svenevs avatar Sep 01 '18 01:09 svenevs