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

Allow blt:init:shell-alias install confirmation to be configurable when run with --no-interaction #3937

Merged
merged 3 commits into from
Dec 9, 2019

Conversation

lpeabody
Copy link
Contributor

@lpeabody lpeabody commented Dec 2, 2019

Changes proposed

When issuing blt blt:init:shell-alias --no-interaction, the shell alias should be installed rather than not installed (by default). This could be a configurable value for BC purposes. For automated project setups that run with --no-interaction, this just results in the alias not ever being setup properly.

Steps to replicate the issue

  1. Without the shell alias installed, run blt blt:init:shell-alias --no-interaction.
  2. Run blt --version and note that blt cannot be found.

Previous (bad) behavior, before applying PR

Running blt blt:init:shell-alias --no-interaction does not setup the shell alias.

Expected behavior, after applying PR and re-running test steps

Running blt blt:init:shell-alias --no-interaction sets up the shell alias after setting in blt.yml:

blt:
  alias:
    auto-install: true

Additional details

Previous to BLT 10.x you could run composer run-script blt-alias and this would install the alias for you, which is what our automated setup scripts have used. However on 10.x it's no longer possible to get this behavior since the composer script was removed and the dedicated BLT command does not work in the same manner.

This is a necessary change for automated setups that wish to install the site alias for local developers.
@lpeabody lpeabody changed the title Default BLT alias install command to confirm rather than deny Allow blt:init:shell-alias install confirmation to be configurable when run with --no-interaction Dec 2, 2019
Copy link
Contributor

@danepowell danepowell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks a lot for the PR! I'll forward-port this to 11.x after merging.

@danepowell danepowell added 10.x Backport needed PRs needing to be backported Enhancement A feature or feature request labels Dec 5, 2019
@lpeabody
Copy link
Contributor Author

lpeabody commented Dec 6, 2019

Rad! Figured I'd ask before you merge, but do you think it makes more sense to default blt.alias.auto-install to true or false? Right now I have it set to false for BC purposes, but I think the reasonable default is true. So, do you think it's more important to stick with a BC approach or go with the reasonable default in this instance?

@danepowell
Copy link
Contributor

danepowell commented Dec 6, 2019

I can merge it into 10.x in the BC way, then change the default when I forward-port it to 11.x (which should get a stable release in the next week or two).

@danepowell danepowell merged commit f97a25d into acquia:10.x Dec 9, 2019
danepowell pushed a commit that referenced this pull request Dec 9, 2019
…en run with --no-interaction (#3937)

* Default alias command interactions to yes.

This is a necessary change for automated setups that wish to install the site alias for local developers.

* Allow to set based on configuration, default to false for BC.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backport needed PRs needing to be backported Enhancement A feature or feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants