Refactor WhippetHelpers
snim2 opened this issue · 0 comments
snim2 commented
This issue derives from a conversation on #224.
WhippetHelpers
is a trait that provides a number of capabilities that are used in several Whippet commands. We should refactor it because:
- Classes with names like
ThingHelpers
are a code smell in themselves. The name is not self-documenting and suggest that the class does not adhere to the SRP. - Traits do not make clear the relationships between classes, doubly so when a trait contains state, really they should be mixins.
- We have been inconsistent in how we decide where the top-level directory of a Whippetised repository is. In
whippet_init()
we search for the directory in a way that prevents us from mocking the filesystem. In some commands we don't. We could just check that the CWD containswhippet.json
and is the root a repository and fail otherwise, but we don't always. - We still have some code that searches for
plugins.json
which is related to a previous dependency management scheme that we no longer used. This should be removed. - There are hidden dependencies between methods in the trait. For example,
load_application_config()
assumes thatproject_dir
has been set bywhippet_init()
or something else. This makes it harder for client classes to use the trait.
There are various options for refactoring this trait, but I would suggest something like:
Dxw\Whippet\Files\WhippetJson
and the related class for the lockfile should "find" their related files on disk and should store their own paths.- We should check what is stored in the CWD when Whippet starts, and should fail fast if there isn't a
whippet.json
in the CWN. The whole codebase should assume that the "project directory" is the CWD. - The application config (
config/application.json
) should have a related classDxw\Whippet\Files\ApplicationJson
which creates a default config if none exists. - Filesystem methods such as
check_and_create_dir()
should be in a class of their own, that can easily be mocked in the tests. - The
download_url_to_file()
andunzip_to_folder()
methods should be extracted into a base class that handles APIs.