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 contains whippet.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 that project_dir has been set by whippet_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 class Dxw\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() and unzip_to_folder() methods should be extracted into a base class that handles APIs.
This issue derives from a conversation on #224.
WhippetHelpersis a trait that provides a number of capabilities that are used in several Whippet commands. We should refactor it because:ThingHelpersare a code smell in themselves. The name is not self-documenting and suggest that the class does not adhere to the SRP.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.jsonand is the root a repository and fail otherwise, but we don't always.plugins.jsonwhich is related to a previous dependency management scheme that we no longer used. This should be removed.load_application_config()assumes thatproject_dirhas 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\WhippetJsonand the related class for the lockfile should "find" their related files on disk and should store their own paths.whippet.jsonin the CWN. The whole codebase should assume that the "project directory" is the CWD.config/application.json) should have a related classDxw\Whippet\Files\ApplicationJsonwhich creates a default config if none exists.check_and_create_dir()should be in a class of their own, that can easily be mocked in the tests.download_url_to_file()andunzip_to_folder()methods should be extracted into a base class that handles APIs.