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

Simplify webpack and codebase #495

Merged
merged 4 commits into from
Oct 25, 2021
Merged

Conversation

choldgraf
Copy link
Collaborator

@choldgraf choldgraf commented Oct 9, 2021

The goal of this PR is to boost maintainability of this theme a little bit, it does a couple of things:

  • Consolidates our webpack config into a single file rather than splitting between dev and prod
  • Uses sphinx-autobuild in our nox -s docs-live environment to auto-gen the docs
  • Removes two deprecated functions from our init.py
  • Documents using nox without the virtual environment

Longer term, I would be a fan of simplifying our webpack story even further (potentially removing it entirely). While it is quite useful in the javascript world, I'd be a fan of removing our vendor dependencies in general and only including stuff that we really need by manually adding it into a stc/vendor folder so that it's easier to notice. I think we have to assume the average maintainer of this repository is not familiar with JS workflows, and our current developer tools are already pretty complex for somebody with this background.

closes #468

@@ -6,6 +6,7 @@ pytest
pytest-regressions
beautifulsoup4
sphinx-sitemap
sphinx-autobuild
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically this file is all development dependencies + all additional doc dependencies (required to build the docs)?
(my first reaction was: you don't need sphinx-autobuild to build the docs, but you neither need pytest for that, so the naming/location of the file is a bit confusing I suppose)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep - maybe another approach would be to define dev dependencies in the install itself as an extras_require?

I'm also find just hard-coding sphinx-autobuild in the install for the relevant nox job.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep - maybe another approach would be to define dev dependencies in the install itself as an extras_require?

The main problem (for my personal workflow) is that this doesn't work with conda .. (so for that I still prefer to have a file listing all those optionals/development deps)

Now, this is an existing issue. So merging here as is.

Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Gave this a quick test drive locally, and seems to be working nicely (and better than what we currently have)

@jorisvandenbossche
Copy link
Member

Longer term, I would be a fan of simplifying our webpack story even further (potentially removing it entirely). While it is quite useful in the javascript world, I'd be a fan of removing our vendor dependencies in general and only including stuff that we really need by manually adding it into a stc/vendor folder so that it's easier to notice. I think we have to assume the average maintainer of this repository is not familiar with JS workflows, and our current developer tools are already pretty complex for somebody with this background.

Sounds good to me. I suppose the build aspect of webpack can be replaced with https://github.com/executablebooks/web-compile?

@jorisvandenbossche jorisvandenbossche merged commit 71bd290 into pydata:master Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

yarn build:dev builds multiple times
2 participants