Reduce "global_common" coupling by moving methods that could be static onto "semantically-closer" py files #203
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
static methods that are used application-wide should belong to the utilities python file. inspired by @gmarmstrong's PR #166 on refactoring global_common methods to be static and have a dzutil.py
- display_banner() was only displayed in CLI mode so it makes sense for it to be in the CLI. - get_version(), was mvoed to util since it is a static function that is needed in multiple parts of the application.
ocr_languages can be treated as just a json file instead of being in global_common. This way it is easier to maintain and makes global_common cleaner.
Container-specific methods in global_common class were basically static methods. So it made sense to move these to container.py
The logic for detecting if we were are running on docker or podman and identifying its respective binary were scattered across the codebase. This centralizes it all in container.py
Container-related methods recently moved to container.py no longer
need to have 'container' in their name as they are within the
container scope already.
Additonally it made it awkward to call from another module:
from .. import container
container.get_container_runtime()
Closes #122 as this was the last remaining hardcoded docker reference where the code also applied to podman.
|
@gmarmstrong let me know what you think. I have limited the scope of this PR to moving things out of global common. |
Note: the container installation failure is not addressed here. See #193
|
Thanks for the review @gmarmstrong! |
deeplow
added a commit
that referenced
this pull request
Sep 15, 2022
Reduce "global_common" coupling by moving methods that could be static onto "semantically-closer" py files. Based on work initially made by @gmarmstrong on PR #166: - moves container-specific code out of global_common.py and into container.py - creates a util.py for static methods used through the whole app - move banner code from global_common onto cli.py given that it's only displayed there - updates tests to reflect these changes - move ocr_languages from global_common onto its own json file in share/ocr-languages.json to simplify global_common logic
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on work initially made by @gmarmstrong on #166:
global_common.pyand intocontainer.pyutil.pyfor static methods used through the whole appglobal_commonontocli.pygiven that it's only displayed thereocr_languagesfromglobal_commononto its ownjsonfile inshare/ocr-languages.jsonto simplifyglobal_commonlogic