Replace utils module #666
Replace utils module #666
Conversation
Having a utils module is a code smell in most cases, including this one. The included functionality can be logically distributed to new and existing modules.
This is where all console related code will live. It wraps Console and Prompt methods.
This is where everything that is checked, installed or initialized before the app is built/exported will live.
This is where everything regarding string and object formatting happens.
This is where import magic happens
Every path-related functionality will live here, e.g. join()
Everything regarding processes/ports will happen here
Custom types and type-related functions will live here, e.g. _issubclass()
This is where every functionality egarding actual building and exporting of the app lives
This is where everything regarding execution of the app happens
This can live here, can't it?
Seems like a logical fit
There was a problem hiding this comment.
Thanks for breaking this up - that file was getting big and had many circular imports.
Could we make the following changes:
- Create a
pynecone/utilsdirectory where all these new files will go under - Move the new files into the new folder e.g.
pynecone/utils/format.pyandpynecone/utils/imports.py - In the files that import these, import the module rather than the individual functions - for example
from pynecone.utils import formatand then call the functions likeformat.format_state
Also renamed two modules: run -> exec, path -> path_ops
Forgot to check tests for errors resulting from new import strategy
|
Done, that probably won't fix the workflow error from the first run though. I'll dig into that. |
Seems like it was never needed here until now
|
Seems like |
|
Forgot to lint the docstrings 🤦♂️. Sorry for the delay, should work now. |
|
Thanks for fixing the build! We're cutting a release tonight, I'll merge this in afterwards. |
|
Looking into why the integration test is failing |
|
Talked with @ElijahAhianyo offline - looks like this is because we need to update pcweb and pynecone at the same time. I ran the integration test manually and it passed, so I'll merge this in. |
All Submissions:
Having a
utilsmodule is considered a code smell in most cases, including this one. It did way too much different stuff and even required circular imports in some places. This PR distributes the code formerly contained in theutilsmodule in a logically coherent manner into new and existing modules. It also includes minor fixups that were necessary to pass all tests; the most notable one being the clear separation of web directory creation, frontend package installation and frontend setup. This fixup should increase testability in general, as the frontend setup called the frontend package installation function in the previous code, which made it way harder to mock said installation.Type of change
Refactoring
Changes To Core Features: