executablebooks / sphinx-panels

A sphinx extension for creating panels in a grid layout
https://sphinx-panels.readthedocs.io
MIT License
83 stars 20 forks source link

🐛 FIX: Boostrap -> Bootstrap in code/docs #44

Closed choldgraf closed 3 years ago

choldgraf commented 3 years ago

I noticed in the docs that there seemed to be a typo for the bootstrap config, it was called boostrap instead of bootstrap.

But then when I looked at the code, I saw that the config was also boostrap and the docs were technically correct.

So this PR changes boostrap to bootstrap in the config and docs, assuming that this was a typo. Is that right @chrisjsewell ?

NOTE: when this changes we'll also need to do the same thing in Jupyter Book...I dunno how we missed this one.

This makes me think we may actually need to run a deprecation cycle so that users aren't jarred by the change. @chrisjsewell what do you think?

welcome[bot] commented 3 years ago

Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.
Welcome to the EBP community! :tada:

choldgraf commented 3 years ago

@chrisjsewell I'm not sure what to do here re: deprecation cycle. I'll go with whatever you prefer. Lemme know if you want me to add a deprecation note to this 👍

choldgraf commented 3 years ago

OK I went ahead and added a Sphinx warning if add_boostrap_css is there. This is ready for review/merge whenever @chrisjsewell

chrisjsewell commented 3 years ago

crazy I/we didn't notice this before 🤦

choldgraf commented 3 years ago

I know - I only noticed it after several weeks of having it called boostrap in my own docs too. I'll make the tests pass and ping you but need to deal with a crying baby for a bit

choldgraf commented 3 years ago

OK crying baby was not crying for long 😅

new default is None and the over-writing / CSS excluding only happens if we have an explicit value set 👍

choldgraf commented 3 years ago

while I was at it I updated the documentation to make the config key a separate code block so it's easier to find.

chrisjsewell commented 3 years ago

After merging, you should try running https://github.com/executablebooks/sphinx-panels/blob/master/git_rebase_theme_branches.sh

This cycles through all the theme branches and rebases them on to master (i.e. incorporating the latest commit(s)), which then triggers the RTD build for each. Literally the only difference between these branches should be the default theme name on this line: https://github.com/executablebooks/sphinx-panels/blob/30a28b7acbd19fcccdba5417fa883cf3c0fdd2b4/docs/conf.py#L68

This step should actually be added to the dev instructions

choldgraf commented 3 years ago

Happy to give that a shot. Alternatively if you wanna add a section to the dev-docs with those instructions / what they do / etc, I am happy to follow those steps on my own to make sure that they work and give feedback in the devdocs PR?

welcome[bot] commented 3 years ago

Congrats on your first merged pull request in this project! :tada: congrats
Thank you for contributing, we are very proud of you! :heart:

chrisjsewell commented 3 years ago

Nah just give it a go and I'll do the instructions after: the instruction is literally just open a terminal and run ./git_rebase_theme_branches.sh, that should do it 🤞

sphinx-panels recipe has also just been merged into conda, so once that builds I can add the badge for that at the same time