hechoendrupal / drupal-console-extend-plugin

Drupal Console Extend Composer Plugin
131 stars 31 forks source link

Find a more relevant name for 'uninstall' #18

Open marcelovani opened 6 years ago

marcelovani commented 6 years ago

I am not sure if I understand the use of the uninstall tag As far as I understand, I need to tag my extended commands with bootstrap: uninstall in order to my commands be discovered. I was wondering if we could come up with a more intuitive name for it, some ideas here:

I kind of like the first two, but open for more ideas

Also, need to rename this file accordingly https://github.com/hechoendrupal/drupal-console-extend-plugin/blob/master/src/Extender.php#L88

tashaharrison commented 6 years ago

A couple of extra ideas:

marcelovani commented 6 years ago

type:orphan 🤣

jmolivas commented 6 years ago

require-bootstrap: false

davidgrayston commented 6 years ago

type:launcher?

jmolivas commented 6 years ago

Instead of using a boolean flag type require-bootstrap: false how about keep bootstrap and have as valid options.

marcelovani commented 6 years ago

I like the idea of having multiple values, just throwing another names: We call it scope, which is the scope of the command and can be:

** What would be and example of usage?

Also, would we rename this file or maybe delete it and keep only extend.console.services.yml? See https://github.com/hechoendrupal/drupal-console-extend-plugin/blob/master/src/Extender.php#L88

jmolivas commented 6 years ago

The idea is to replicate what we have on the annotation

https://github.com/hechoendrupal/drupal-console/blob/master/src/Annotations/DrupalCommand.php

<?php
/**
 * @file
 * Contains \Drupal\Console\Annotations\DrupalCommand.
 */

namespace Drupal\Console\Annotations;

use Doctrine\Common\Annotations\Annotation;
/**
 * @Annotation
 * @Target("CLASS")
 */
class DrupalCommand
{
    /**
     * @var string
     */
    public $extension;

    /**
     * @Enum({"module", "theme", "profile", "library"})
     */
    public $extensionType;

    /**
     * @var array
     */
    public $dependencies;

    /**
     * @Enum({"none", "site", "install"})
     */
    public $bootstrap;
}

We use the annotation like this https://github.com/hechoendrupal/drupal-console/blob/master/src/Command/Features/ImportCommand.php :

<?php

 ... 

use Drupal\Console\Annotations\DrupalCommand;
use Drupal\Console\Core\Command\Command;
/**
 * @DrupalCommand(
 *     extension = "features",
 *     extensionType = "module"
 * )
 */
class ImportCommand extends Command
{
jmolivas commented 6 years ago

If no bootstrap set on the command annotation the is defaulted to install you can check the DrupalCommandAnnotationReader class here => https://github.com/hechoendrupal/drupal-console/blob/master/src/Annotations/DrupalCommandAnnotationReader.php

jmolivas commented 6 years ago

I think the site-install name instead of install. Makes sense as well.

Not sure about global since the name of the option is bootstrap and is related to the bootstrap level of the site. I think none make more sense than global.

jmolivas commented 6 years ago

We can probably use global but we need to rename annotation property from bootstrap to scope which I think will work as well.

Then we can use the names as @marcelovani suggested https://github.com/hechoendrupal/drupal-console-extend-plugin/issues/18#issuecomment-344000715

marcelovani commented 6 years ago

Thanks for explaining. I reckon, if these annotations are not documented, not many people are using it, so it would be a good time, if we want to rename. And of course, we need to be consistent with annotations, file names, etc. Quick question about https://github.com/hechoendrupal/drupal-console/blob/master/src/Annotations/DrupalCommandAnnotationReader.php#L24, should not that be 'install' in this case?