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

html_theme_path docs out of sync #186

Open
svenevs opened this issue Aug 26, 2018 · 4 comments
Open

html_theme_path docs out of sync #186

svenevs opened this issue Aug 26, 2018 · 4 comments
Assignees

Comments

@svenevs
Copy link
Collaborator

svenevs commented Aug 26, 2018

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 svenevs self-assigned this Aug 26, 2018
@ryan-roemer
Copy link
Owner

@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!!!

@svenevs
Copy link
Collaborator Author

svenevs commented Aug 28, 2018

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.

@ryan-roemer
Copy link
Owner

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)...

@svenevs
Copy link
Collaborator Author

svenevs commented Sep 1, 2018

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

Hmm. Very intriguing. This can definitely be automated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants