drupal-composer / drupal-project

:rocket: Composer template for Drupal projects. Quick installation via "composer create-project drupal-composer/drupal-project"
GNU General Public License v2.0
1.56k stars 941 forks source link

Move drupal/console from a required dependency to a dev dependency #352

Closed pfrenssen closed 6 years ago

pfrenssen commented 6 years ago

Drupal Console has some nice functionality but it is not strictly required for drupal-project to function.

I would propose to move it from the require section to the suggest section. This means that when a Drupal project is created or updated, Composer will output a suggestion to install Drupal Console, and developers can then decide whether to use it or not depending on the needs of their project.

jackbravo commented 6 years ago

But the same would apply to drush right? Neither are actually needed for creating a drupal site, but people use them extensively in development. Since this is a starting point primarily for developers I like that both are enabled.

FatherShawn commented 6 years ago

And I noted that we should treat them similarly on the PR at https://github.com/drupal-composer/drupal-project/pull/353#issuecomment-365975028

derhasi commented 6 years ago

I personally see drupal/console still as a best-practice to use in a drupal development setting. As this project should be a collection of best-practices I would not strip that away. For me Drupal Console and Drush are a crucial part of Drupal development, especially when you are used to go with the CLI and therefore they should be there out of the box in my opionion.

g3r4 commented 6 years ago

I mean, if you are a professional developer that does not like to waste time, I would say it is required. I don't see the harm in including it, only benefits. Also, Symfony comes with Symfony console as part of the project, Why shouldn't Drupal come with something similar (but more powerful)?

eliasdelatorre commented 6 years ago

For the sake of consistency, both Drush and Console need to be moved to "suggestions", since none of them are strictly required for a Drupal installation.

I can see why Drush may be set as a required dependency but, frankly, the same applies to Console. Since the D8 workflow is much more complex, there's a need to have enough tools to make the initial development doable.

If this issue is about strict syntax and low level discussions, then ok, let's remove it, but we need to do the same for everything else that is not required, just like Drush.

Personally I feel that both are required if you want to do any actual work with D8.

nicolas-grekas commented 6 years ago

This doesn't provide the same experience at all. drupal-console is a requirement to the experience the authors of drupal-composer want to provide out of the box. Thus it belongs to "require". I understand that you have a different definition of what "require" means, but there is nothing anywhere that makes your definition more accurate than this definition given here: a requirement to the experience. That's a great thing.

allgood2386 commented 6 years ago

The use case for having both tools installed by default is a far saner default then catering to the limited use case where you can't have it (presumably for infra/ATO/FEDRAMP compliance type deals).

I'd argue as a project which is providing a default code base to work from it makes sense to keep both projects in the required section. If someone needs to remove them it should be 2 trivial composer commands.

That all being said, if it is moved, both tools should be treated equally.

pfrenssen commented 6 years ago

Everybody seems to be missing the point a bit here. Composer is a dependency management tool with very well defined sections. I'm not advocating to remove drupal-console, but to move it to the right section. Currently it is in require which is not correct, because a Drupal project does not require it to function in production. require is the section that declares the dependencies for production.

@jackbravo

Neither are actually needed for creating a drupal site, but people use them extensively in development. Since this is a starting point primarily for developers I like that both are enabled.

@derhasi

I personally see drupal/console still as a best-practice to use in a drupal development setting. As this project should be a collection of best-practices I would not strip that away. For me Drupal Console and Drush are a crucial part of Drupal development, especially when you are used to go with the CLI and therefore they should be there out of the box in my opionion.

@eliasdelatorre

I can see why Drush may be set as a required dependency but, frankly, the same applies to Console. Since the D8 workflow is much more complex, there's a need to have enough tools to make the initial development doable.

What all of you have in common is a need is to have drupal-console on a development environment, so that you can create a drupal site and develop it: then require-dev is the place to be. Not require which is the production build.

I wanted to add it to suggest so people can decide for themselves where to put it. But require-dev is also fine for me. I personally use drupal-console for initial scaffolding at the start of the project, and after this phase I don't need it to be permanently available in my development environment. But it seems most people here think it is worthwhile to have it permanently on their dev machines.

pfrenssen commented 6 years ago

@eliasdelatorre

For the sake of consistency, both Drush and Console need to be moved to "suggestions", since none of them are strictly required for a Drupal installation.

We cannot move Drush to suggest since it is an actual dependency of drupal-project itself, we use it in the test suite. I proposed on the PR to move it to require-dev but @weitzman made the correct observation that a typical Drupal site requires it in the production environment.

jmolivas commented 6 years ago

@pfrenssen We used and have clients using DrupalConsole on production servers and also using it on their CI/CD workflow.

All this is because:

Making a change like this will affect people using DrupalConsole via DrupalComposer on prod sites for daily site maintenance and debugging.

pfrenssen commented 6 years ago

@jmolivas yes but you are the maintainer, you are a special snowflake :) I think 80% of Drupal sites do not require it in production.

It is a heavy package with many dependencies. For maintenance reasons the production builds should be as small as possible.

jmolivas commented 6 years ago

Just because you used only for development it does not mean everyone uses it like that.

I wanted to add it to suggest so people can decide for themselves where to put it. But require-dev is also fine for me. I personally use drupal-console for initial scaffolding at the start of the project, and after this phase I don't need it to be permanently available in my development environment. But it seems most people here think it is worthwhile to have it permanently on their dev machines.

jackbravo commented 6 years ago

so... @weitzman (drush maintainer) made an observation that drush is used in production in any typical web site, and you assume that as correct. @jmolivas makes the same observation for drupal console, but he is a special snowflake.

jackbravo commented 6 years ago

About drupal console being a heavy package. Both drush and console have a similar number of dependencies:

Back when both used to be released as .phar packages drush was 4.38 MB and console was 2.25 MB.

FatherShawn commented 6 years ago

As I stated in https://github.com/drupal-composer/drupal-project/issues/357#issuecomment-365992646 the related issue. The purpose of this project is not to run Drupal but to manage a Drupal site and its dependencies from the command line. We need all the CLI tools to do that well.

pfrenssen commented 6 years ago

@jackbravo

so... @weitzman (drush maintainer) made an observation that drush is used in production in any typical web site, and you assume that as correct. @jmolivas makes the same observation for drupal console, but he is a special snowflake.

I don't know what you are trying to achieve with this comment, but I suppose you are upset about my apparent plan to "demote" your project. FYI I don't base my information on a single comment from a single person, and I hope you are not extrapolating your own decision making process onto me.

Let me explain a bit better why I think drupal-console should be removed as a default option from require, but it is OK for Drush to stay. Let it be clear that I have nothing against drupal-console, on the contrary, I have created dozens of contributions to it, and continue to do so.

Now that a composer based workflow has become the standard way of working, drupal-project is going to be the base of a great many Drupal sites in the future. Drupal is used for very serious projects, for government sites and Fortune 500 companies, for thousands of e-commerce sites, but also smaller sites that are crucial for the operation and survival of tens of thousands of companies all over the world.

We as developers have to be very careful about the software we put on our clients' servers. After all, a bug in them might cause the websites to go offline, or the servers compromised. I was in fact motivated to create this issue because the latest release of drupal-console introduced a new dependency on symfony/thanks as a hard dependency. This is a faux-pas from a professional standpoint, we shouldn't deploy software like this on the servers of our clients. It makes us look as if we care more about Github stars than about the stability of our client's servers.

So to get to the point, I think drupal-console in its current state is not ready yet to be deployed as the default option for the following reasons:

In contrast:

If drupal-console wants to be taken seriously as a production CLI tool, it should start requesting test coverage for new PRs that deal with the commands related to production maintenance commands, and it should retroactively add missing coverage.

For the moment, drupal-console has proven its worth as a dev tool, so putting it in require-dev is fine by me. People are also free to add it in their project's production build if they want to.

cmcintosh commented 6 years ago

I would like to commend @pfrenssen for removing drupal-console from production, actually, I stopped using it when the symfony/thanks package was added as a dependency. I have seen the same thing in drupal modules and its a dangerous slope.

gnuget commented 6 years ago

The code base is too young. It has been only 5 months since the stable 1.0.0 version was released.

Under the same argument, we shouldn't recommend using Drush 9 (it only has passed a month since it was released) or event this project because it only has 3 years since the initial comment, yes?

Drupal Console has almost 2 million installations and it has a ton of users and has been under development for 5 years, I wouldn't consider it that is not mature.

I was in fact motivated to create this issue because the latest release of drupal-console introduced a new dependency on symfony/thanks as a hard dependency.

To be honest, the timing with this issue and this one and this tweet make this to look like if you are now trying to get a vendetta against the DrupalConsole. :-/

Wouldn't be better to help the console to write those so much needed tests and leave the project where it is now?

A lot of users are going to be affected if this project is moved to the require-dev or to suggest

pfrenssen commented 6 years ago

I have been considering this for a while, but the adding of symfony/thanks was indeed the proverbial drop in the bucket that caused me to take action, because it directly affects all of our current user base of drupal-console. I feel a bit responsible for this since I should have taken action earlier. I suppose I'm now trying to do some kind of damage control.

I don't feel I can recommend using drupal-console on production for our entire worldwide Drupal user base for the reasons I outlined above. My experience is mainly with the code generation part of drupal-console, and I have only used the site maintenance features a few times to try them out. I am sure the site maintenance commands work well for many people on a day to day basis, but it doesn't feel safe to me because the project moves so fast and without coverage.

Please understand that having drupal-project in the require section of drupal-project is like an official endorsement from us that we approve it to be production ready. I am feeling a huge weight of responsibility for this. What if something breaks in a future update that affects tens of thousands of sites? I am dealing with stone faced devops and security analysts on a daily basis and they are very conservative about what is allowed on the production servers.

If there would be test coverage for the mission critical maintenance commands this would give me much more confidence, but also the projects needs some time to mature I think.

jmolivas commented 6 years ago

@pfrenssen

It is a heavy package with many dependencies. For maintenance reasons the production builds should be as small as possible.

Talking about dependencies DrupalConsole add 23 and Drush 26. Which means Drush add more dependencies to your production build.

This is a faux-pas from a professional standpoint, we shouldn't deploy software like this on the servers of our clients. It makes us look as if we care more about Github stars than about the stability of our client's servers.

The symfony/thanks was relocated as suggested at DrupalConsole https://github.com/hechoendrupal/drupal-console/pull/3789, we will follow this issue at d.o. https://www.drupal.org/project/drupal/issues/2945263

The code base is too young. It has been only 5 months since the stable 1.0.0 version was released.

Drupal Console 1.0 stable has only 5 months released but has been tested for more than two years now on Drupal 8 sites.

Drush has been used in production for more than 10 year

Yes, Drush has almost 10 years but that was the legacy version right. Drush 9 was a total rewrite using OOP, Symfony components, and third-party dependencies and the stable 9.0 version was released. less than a month ago.

There is no proper test coverage.

We totally agree on this one. This is must-have for the project and we are working on having fixed we are defining a roadmap and the ETA for this issue.


Once we have the test coverage for the mission-critical maintenance commands. Would you reconsider this resolution and bring it back as require dependency?

jmolivas commented 6 years ago

Related to @gnuget comment

Wouldn't be better to help the console to write those so much needed tests and leave the project where it is now?

Anyone is welcome to contribute to DrupalConsole ;)

jackbravo commented 6 years ago

I don't know what you are trying to achieve with this comment, but I suppose you are upset about my apparent plan to "demote" your project. FYI I don't base my information on a single comment from a single person, and I hope you are not extrapolating your own decision making process onto me.

Sorry for being a little rude. I was trying to make you see how someone who didn't have all the background that you just laid out (like me) might see your comments. What I was trying to point is that I don't think anyone has an exact metric on how much is drush 9 used in production against drupal console. Without all the background it seems like treating two projects with different standards based on previous interactions with each project. I will disclose that I've known Olivas for a long time and consider him a friend. But also I don't have anything against moshe, and in fact I also consider him in high regard from even before drush existed. He probably doesn't remember me, but I've spoke with him in the past, he speaks a little spanish as well :-p.

So, anyway. Reading your points, I would agree on having drupal console on require-dev until the test coverage is improved. I would say that drush 8 is fine in require, but we are talking about drush 9 here and I don't know how their test coverage is, but it is a complete rewrite so it should also be evaluated as part of this issue.

jackbravo commented 6 years ago

So, just to clarify it further. Without the background you just laid out. All I could see was one comment from drush maintainer saying that it is indeed used in production and you responding "yes I agree". And another comment from drupal console maintainer saying that it is used in production and you responding "you are a special snowflake, of course you use it in production, but few other people do".

jmolivas commented 6 years ago

@cmcintosh Can you point where I did attack others for choosing to develop and contribute things in a way I 'deem' inappropriate.?

gnuget commented 6 years ago

Yup, I'm not sure what you are talking about if I've been rude it wasn't my intention.

I just shared my impression about this issue I never wanted to be disrespectful.

jmolivas commented 6 years ago

@cmcintosh totally out of the scope of this thread but to add some context

Interesting how you complain about my reply but said nothing about the original tweet from joachimnicolas

I briefly used it for its entity type code generator... then I wrote a better one in Drupal Code Builder :D

Mentioning you wrote something better without providing a real comparative and data

I am not interested to resurrect that discussion. That thread is closed already with a great comment by NickWilde.

Contribution to existing projects and trying out new ways are both valuable and can serve different purposes and lead to improvement.

gnuget commented 6 years ago

not sure how I went going after to joachimnicolas with my tweet.

My question was honest, if his generator is better we could re-use some of that to improve the console, sorry if it sounds rude to you.

But agree that is out of the scope of what we are discussing here, you aren't helping to move forward this discussion to be honest.

pfrenssen commented 6 years ago

Thanks everybody for the constructive discussion.

I would be happy to add drupal-console back to the require section when the automated tests are running and the "mission-critical" commands are covered with test. What I mean with "mission-critical" are the ones that are absolutely essential for a devop to investigate a problem on production and deploy updates: clearing the cache, performing database updates, enabling / uninstalling modules etc. I don't care about tests for pure development related commands like code generation etc.

I also think that there will be very little impact for the main target audience of drupal-console (developers) if we move it to require-dev. All developers install the dev dependencies anyway, so they will have it available. Also we don't impact existing users of drupal-project - the projects are created using composer create-project which doesn't create an upstream reference back to the project. Any changes we make are not pulled back into the projects of our users. So drupal-console will not suddenly vanish from our existing users' production instances.

I don't know if you have a specific time frame in mind for adding test coverage? Maybe we can revisit it in 3 or 6 months?

I will leave this open for a few more days in case this requires some more discussion, but I think it looks we are mostly in agreement now.

FatherShawn commented 6 years ago

@cmcintosh Your comment above was inappropriate: https://github.com/drupal-composer/drupal-project/issues/352#issuecomment-367533450 because it does not discuss the presented issue but shifts to an ad hominem strategy or personal attack. Behaving in this way is damaging to our community. Specifically, such behavior undermines the technical discussion at hand and discourages others from contributing or speaking up for fear of being targets themselves.

cmcintosh commented 6 years ago

@FatherShawn Im on slack if you wnat to discuss further

pfrenssen commented 6 years ago

Let's please keep this on topic. I know I am to blame for the emotions here since it was too harsh to propose to move drupal-console to suggest instead of require-dev. I apologise for upsetting anyone.

Let's just agree that it is difficult to communicate on the internet and people just care very deeply about their work, we're just humans! Please don't criticize people here for what they have said. Let's focus on the software :)

FatherShawn commented 6 years ago

@cmcintosh It is out of line for the following reasons:

  1. The disagreement under discussion is the technical merits of their code and the appropriate use of their code in this project. It is off topic.
  2. Personal attacks in public damage community and individuals. Which is why it is addressed in the Drupal Code of Conduct.

@pfrenssen We need to keep this conversation on topic, agreed. But not saying something to each other when we depart from the appropriate conduct is also not correct - it gives consent to the behavior.

cmcintosh commented 6 years ago

Did you even read the thread on twitter, or are you just jumping on a bandwagon? The thing you are accusing me of is exactly what was happening and it was in relation and referenced on this issue. I have personally been the target of such comments/attacks and is why I spoke up. There is nothing more chilling to contributing to being told, you shouldn't be working on the module you are working on because it is too similar to something else.

cmcintosh commented 6 years ago

unfollowing this thread as it will just lead to me getting more heated and being further misunderstood. Im available in other more appropriate channels. Towards the moving of console to require-dev / suggest i think its a valid idea. There is no need to have a development tool on production, it is similar to having Devel turned on.

jmolivas commented 6 years ago

@cmcintosh But DrupalConsole is no longer development tool only, it is fully featured CLI that why is used in production as mentioned before you can use it to import/export config, install drupal, clear cache, debug routes, debug services. Yes, the code generation is probably the part that shines the most, but is no longer a scaffolding tool only, is a Full CLI to administer the system to automate tasks creating pipelines and execute several commands using YAML definitions.

gnuget commented 6 years ago

There is no need to have a development tool on production, it is similar to having Devel turned on.

Yup, this means that you aren't using all the features of the tool.

I don't know if you have a specific time frame in mind for adding test coverage? Maybe we can revisit it in 3 or 6 months?

I will leave this open for a few more days in case this requires some more discussion, but I think it looks we are mostly in agreement now.

This sounds good to me. Thanks!

jmolivas commented 6 years ago

I also think that there will be very little impact for the main target audience of drupal-console (developers) if we move it to require-dev

DrupalConsole is no longer development tool only, it is fully featured CLI that why is used in production. But will impact people using it to deploy their sites.

I would be happy to add drupal-console back to the require section when the automated tests are running and the "mission-critical" commands are covered with test.

We totally agree on this one. This is must-have for the project, and we are working on having fixed we are defining a roadmap and the ETA for this issue.

I don't know if you have a specific time frame in mind for adding test coverage? Maybe we can revisit it in 3 or 6 months?

We will keep you informed.

I will leave this open for a few more days in case this requires some more discussion, but I think it looks we are mostly in agreement now.

Fair enough

derhasi commented 6 years ago

Thanks for getting back on the topic. I know it is sometimes hard to perceive the perspective of others, especially if they seem to be or are emotional.

On the topic: I am not sure on the next move now. I'd tend to move drupal/console to require-dev, because of my personal usage behaviour, but I also can see the valid use case of a production replacement/fallback for drush.

I think @webflo should review this for a "final" decision.

webflo commented 6 years ago

Wow, this was a heated discussion.

I will keep it as is for now. This could be a feature of the setup wizard. #384

Thank you all!