jonathan-s / django-sockpuppet

Build reactive applications with the django tooling you already know and love.
https://github.com/jonathan-s/django-sockpuppet
MIT License
450 stars 22 forks source link

Fixes to SocketpuppetConsumer for handling dotfiles #57

Closed mekhami closed 3 years ago

mekhami commented 3 years ago
1. Update requirements_dev.txt to include base requirements, as
   these are necessary for starting the server and performing any tests.

2. Use Reflex.__subclasses__() rather than name checking for reflex
   subclass detection, which will be more consistent and allow users
   to name their reflexes whatever they want.

3. Clear falsey values from the filepaths detected in the
   load_reflexes_from_config function, which is a tenuous solution to
   not including vim-generated dotfiles in the module loading process.

Type of PR (feature, enhancement, bug fix, etc.)

Bugfix and enhancement

mekhami commented 3 years ago

To be clear, the solution here for dotfiles is not a solution, just a bandaid. It's a symptom of the larger problem of making big assumptions about where developers are going to put their code. It would be better if we assumed the presence of a reflexes module, whether that's a reflexes.py or a reflexes directory with an __init__.py that imports all of its submodules. That would simplify discovery, and not have to parse file paths in directories anymore.

But I didn't want to put that assumption into this PR; if it's something the maintainer(s) can get behind, I'm happy to make that change as well.

DamnedScholar commented 3 years ago

Assuming the presence of a reflexes module is close enough to the existing behavior to be identical in most cases and is more Pythonic, so I like that idea.

jonathan-s commented 3 years ago

@mekhami I'd support such a PR, and thanks for taking the time to resolve this!