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

Simplify Command boilerplate for typical use-cases #218

Merged
merged 16 commits into from
Sep 24, 2024

Conversation

totten
Copy link
Member

@totten totten commented Sep 24, 2024

Overview

This is a general shuffling the Command class hierarchy. The basic Command from symfony/console is the same, but several of the helpers used by cv commands have been moved around.

Most notably, the BaseCommand (internal to cv-app) has been replaced by the CvCommand (which is now part of cv-lib). The contract for CvCommand should be easier to use, and -- when we get CiviCRM extension integration (#219) -- it should slot-in more easily.

Before

  • The BaseCommand class is internal to cv-app. It is fairly heavy with helpers that don't need to be in there (low cohesion).

  • Pretty much all commands require the use of BootTrait. The boilerplate code involves importing the trait, activating the trait, and then figuring out where to call a few helper functions. This is done in every command-class.

After

  • The CvCommand class is part of cv-lib, and it includes BootTrait.

  • Some other helpers have been moved out to standalone utilities.

  • Common bootstrap options (--level, --user) are declared in BaseApplication. Bootstrap behavior is automatically included by way of CvCommand. The default behavior assumes that you want Civi to be booted before running the command. But this can be overridden via getBootOptions().

Comments

  1. This can be viewed as a code-cleanup on the existing commands.
  2. This can be viewed as a contract-break for the command classes. However, there rare mitigating factors: plugins.md referred to this part of the functionality as experimental. Plugin support was only recently merged. So... I don't think there's much of an install-base needing the old contracts of BootTrait.
  3. I'm working toward a subsequent PR where cv commands are loaded from civicrm extensions. The thing is that... civicrm extensions are only loaded post-boot. The options like --level and --user should still work, but they can't work the same. (The Command class hasn't been loaded... so we have to start processing those flags before have the Command.)

@totten totten changed the title Simplify Command pattern for typical use-cases Simplify Command boilerplate for typical use-cases Sep 24, 2024
@totten totten merged commit fa71612 into civicrm:master Sep 24, 2024
1 check passed
@totten totten deleted the master-cmd-ref branch September 24, 2024 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant