dxw/whippet

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:

  1. 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.
  2. Traits do not make clear the relationships between classes, doubly so when a trait contains state, really they should be mixins.
  3. 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.
  4. 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.
  5. 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.