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

Refactor source code and split it up into multiple files? #162

Open
agriyakhetarpal opened this issue Mar 29, 2024 · 4 comments
Open

Refactor source code and split it up into multiple files? #162

agriyakhetarpal opened this issue Mar 29, 2024 · 4 comments

Comments

@agriyakhetarpal
Copy link
Contributor

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 agriyakhetarpal added the enhancement New feature or request label Mar 29, 2024
@steppi steppi added maintenance and removed enhancement New feature or request labels Mar 29, 2024
@steppi
Copy link
Collaborator

steppi commented Mar 29, 2024

cc: @steppi

Thanks @agriyakhetarpal, but I don't really have an opinion on this one way or the other, beyond that I agree it would be good to add a CONTRIBUTING.md. I had no experience working on sphinx extensions prior to this and just fit the try_examples directive into the existing project structure. If you want to work on this, I think this should be decided by the other maintainers.

@martinRenou
Copy link
Member

martinRenou commented Mar 29, 2024

Hey! No strong feeling about whether or not to split the package in multiple files. Though could be good indeed if we continue adding features to it.

@agriyakhetarpal
Copy link
Contributor Author

Sure! My suggestion here is quite trivial, really, so I'll be happy to start my work on this once some of the recent PRs can get merged – and maybe after we come out with a release with those, to not cause any trouble(s) before a release.

@martinRenou
Copy link
Member

Sounds great!

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

3 participants