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

Pull functionality out of util.py #737

Closed
micahellison opened this issue Nov 12, 2019 · 2 comments · Fixed by #1027
Closed

Pull functionality out of util.py #737

micahellison opened this issue Nov 12, 2019 · 2 comments · Fixed by #1027
Labels
enhancement New feature or request 📌 This can't go stale

Comments

@micahellison
Copy link
Member

Feature Request

  • What is the motivation / use case for changing the behavior?
    Currently, util.py is a grab-bag of different functionality related to encryption, text coloring, configuration, and retrieving text from an editor. It also contains the colorama statement that makes text coloration possible in Windows terminals.

It would be nice to have each method moved to an appropriate class and ideally, remove util.py entirely so we can adhere to the single responsibility principle.

@micahellison micahellison added the enhancement New feature or request label Nov 12, 2019
@alichtman
Copy link
Contributor

alichtman commented Nov 12, 2019

Agreed. util.py is getting a little too big to work in easily.

I propose splitting it into: encryption.py, printing.py, prompting.py, and config.py. printing and prompting maybe should be combined?

@wren wren added the 📌 This can't go stale label Nov 30, 2019
@wren
Copy link
Member

wren commented Aug 16, 2020

@alichtman The current PR goes very closely with your recommendations here. We ended up putting encryption stuff into EncryptedJournal.py because those functions need to be folded into that class (I think this will let us more easily support different encryption methods like pgp without reaching up into the control flow).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request 📌 This can't go stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants