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

Find a more relevant name for 'uninstall' #18

Open
marcelovani opened this issue Aug 23, 2017 · 11 comments
Open

Find a more relevant name for 'uninstall' #18

marcelovani opened this issue Aug 23, 2017 · 11 comments

Comments

@marcelovani
Copy link
Member

marcelovani commented Aug 23, 2017

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:

  • type: standalone
  • type: global
  • type: extend
  • type: custom
  • require-site: no
  • require-drupal: no

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
Copy link

A couple of extra ideas:

  • type: independent
  • type: autonomous

@marcelovani
Copy link
Member Author

type:orphan 🤣

@jmolivas
Copy link
Member

jmolivas commented Sep 5, 2017

require-bootstrap: false

@davidgrayston
Copy link

type:launcher?

@jmolivas
Copy link
Member

jmolivas commented Nov 13, 2017

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

  • none: Do not require a Drupal site.
  • site: Does require a Drupal site, but the Drupal could be not installed.
  • install: Does require a Drupal site and site must be installed.

@marcelovani
Copy link
Member Author

marcelovani commented Nov 13, 2017

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:

  • global: Does not require a Drupal site (Used to build Drupal sites)
  • site: Requires a Drupal site but does not require Drupal to be installed (Used for ?**)
  • site-install: Requires a site with a Drupal installation (Used for site level commands)

** 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
Copy link
Member

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
Copy link
Member

jmolivas commented Nov 13, 2017

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
Copy link
Member

jmolivas commented Nov 13, 2017

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
Copy link
Member

jmolivas commented Nov 13, 2017

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 #18 (comment)

@marcelovani
Copy link
Member Author

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?

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

No branches or pull requests

4 participants