search

acquia / blt

Acquia's toolset for automating Drupal 8 and 9 development, testing, and deployment.
https://docs.acquia.com/blt/
GNU General Public License v2.0
443 stars 396 forks source link

8.9.1 Feedback #1903

Closed dpagini closed 7 years ago

dpagini commented 7 years ago

My system information:

Just wanted to document a few pieces of feedback after my upgrade to 8.9.1. Please feel free to close this, or I can create individual tickets if any of these items warrant them.

  1. I'm confused about the /phpcs.xml.dist file. This is supposed to live in the root now? This file can't live in vendor, I assume based on how PHPCS works OOTB? Why not edit this file directly for customizations? B/c it will get overridden by BLT?
  2. I have Drupal-VM installed. The BLT doctor command checkComposer:plugins for prestissimo runs inside my VM, but all my composer commands run on my host. Should this be checking the host, or is it correct to check the VM?
  3. The BLT doctor command checkDrupalVm:aliasis reporting an error for me that my local alias is not in drush/site-aliases/aliases.drushrc.php. Is that right for a Drupal-VM user? Should you commit your local Drupal-VM alias to the project? I'm afraid to do that for all the developers not using Drupal-VM who now have this alias that doesn't work.
  4. When I run $ blt setup I have now gotten prompts to drop the existing database, and enable/disable modules. I can manually pass -y flag to that command and it works OK, but I don't think that's the intention. Is this the issue with the vagrant-exec plugin again? I think this might be only real bug here.
  5. The new "shell" doctor output is blank for me. So is "user", but I haven't looked if that's a new item or not... is that expected on Mac maybe?
grasmash commented 7 years ago

I'm confused about the /phpcs.xml.dist file. This is supposed to live in the root now? This file can't live in vendor, I assume based on how PHPCS works OOTB? Why not edit this file directly for customizations? B/c it will get overridden by BLT?

Yes, yes, and yes. Also see https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#using-a-default-configuration-file.

I have Drupal-VM installed. The BLT doctor command checkComposer:plugins for prestissimo runs inside my VM, but all my composer commands run on my host. Should this be checking the host, or is it correct to check the VM?

It should run on host.

The BLT doctor command checkDrupalVm:aliasis reporting an error for me that my local alias is not in drush/site-aliases/aliases.drushrc.php. Is that right for a Drupal-VM user? Should you commit your local Drupal-VM alias to the project? I'm afraid to do that for all the developers not using Drupal-VM who now have this alias that doesn't work.

Committing your alias is the preferred practice. It greatly reduces on-boarding issues, and for those not using a VM, having a non-working alias isn't a nuisance.

When I run $ blt setup I have now gotten prompts to drop the existing database, and enable/disable modules. I can manually pass -y flag to that command and it works OK, but I don't think that's the intention. Is this the issue with the vagrant-exec plugin again? I think this might be only real bug here.

I'm on the fence about what this behavior should be, but I admit that changing it without cause is not ideal.

The new "shell" doctor output is blank for me. So is "user", but I haven't looked if that's a new item or not... is that expected on Mac maybe?

This has no practical ramifications, as it is simply used to provide debugging information. But, I would like to know why that variable is not set.

dpagini commented 7 years ago

Thanks @grasmash. I'll take a look into the last issue if I have time...