jupyterlite-sphinx icon indicating copy to clipboard operation
jupyterlite-sphinx copied to clipboard

Refactor source code and split it up into multiple files?

Open agriyakhetarpal opened this issue 3 months ago • 4 comments

Problem

An issue I found while contributing to the Sphinx extension, coupled with the lack of a CONTRIBUTING.md document (which is another, unrelated issue) is that it was a bit difficult to navigate what was going on for someone whose Sphinx skills have been a bit rusty since quite some time. I would like to put in a request to refactor the codebase and move things around, and here are my suggestions:

Proposed Solution

  1. A complete refactor might take a lot of time and hinder development efforts, but at least moving the IFrame classes to a non-namespace sub-module jupyterlite_sphinx/iframes/ and then similarly moving the directives to jupyterlite_sphinx/directives/ can work
  2. Another level of hierarchy exists with the Directive classes, where all of them are subclasses that derive from _LiteDirective, which itself derives from the standard SphinxDirective. These would be likely candidates to split up into multiple files, and it should be easy to fix up their imports back into jupyterlite_sphinx.py (it might make sense to rename the file because of the folder structure, i.e., jupyterlite_sphinx.main sounds a bit better than jupyterlite_sphinx.jupyterlite_sphinx)
  3. Besides that, I'm not sure if anything else can be extracted out of a refactoring process (the jupyterlite_sphinx.js and jupyterlite_sphinx.css files are standalone in their usage and exist only for copying into the _static folder for a documentation website.

Additional context

Though it is a bit outdated and unmaintained at the time of writing, another project: https://github.com/sphinx-contrib/cookiecutter has a nice structure to offer. However, it may or may not be needed here for now because the packaging is modern with hatch anyway and a test suite does not exist (and that may not be required anytime soon?).

cc: @steppi

agriyakhetarpal avatar Mar 29 '24 12:03 agriyakhetarpal