Skip to content

Conversation

@greg-1-anderson
Copy link
Member

@greg-1-anderson greg-1-anderson commented Oct 17, 2017

Includes tests for both, and documentation in example.aliases.yml.

@weitzman
Copy link
Member

consider adding docs to example.drush.yml and/or example.aliases.yml.

@greg-1-anderson
Copy link
Member Author

Right now I am entombed in the hellish quagmire called backend invoke, which resists divulging the secret of drush-script.

@greg-1-anderson
Copy link
Member Author

Ah. $site_alias_record->legacyRecord() converts things back into the form backend invoke expects, but it leaves off some critical information, like drush-script and ssh-options.

… to set up test sites with only a settings.php + database entries (no installed site) and alias records for testing in --simulate mode.
@greg-1-anderson greg-1-anderson changed the title Inject options from rsync alias parameters. Inject options from core:rsync and sql:sync alias parameters. Oct 18, 2017
Copy link
Member

@weitzman weitzman left a comment

Choose a reason for hiding this comment

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

I'll test this and possibly add more feedback. Thanks for your hard work here (and Consolidation).

My one thought is to pause and ask if we really need this alias-parameters feature. It looks analogous to the source-/target- prefixes of old. That feature was little used. My thinking is that folks can get used to the rule that command specific options will be honored depending where the command runs. That is:

  1. drush sql-sync @foo @bar. local command-specific is honored
  2. drush @bar sql-sync @foo @self. @bar's command-specific is honored

# - 'alias-parameters': These options will only be set if the alias is
# used as the specified parameter. `sql:sync` and `core:rsync` are the two
# core commands that use this entry. These commands both have `source`
# and `target` parameters.
Copy link
Member

Choose a reason for hiding this comment

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

can we add config-pull to this list?

@greg-1-anderson
Copy link
Member Author

1. drush sql-sync @foo @bar. local command-specific is honored

Perhaps in this scenario, if we also took the command-specific options from @foo and @bar, that might be sufficient. I used to use source-command-specific and target-command-specific to handle use cases that config split uses now. With rsync, this feature could be useful to exclude a path when an alias is a target but not the source, or visa-versa.

This feature only takes a small amount of code now, so it shouldn't be too hard to maintain. I'm not convinced that it's critical, but I'm reluctant to give it up. Maybe take a survey and see if it is used?

@bar
Copy link

bar commented Oct 18, 2017

@greg-1-anderson @weitzman please, don't use my handle to document your code :p

thanks

@greg-1-anderson
Copy link
Member Author

Seems like it is safe to use @ without making an @-mention as long as the reference is inside of a code block. If we always embed alias references in code blocks, then we won't need to worry about sending spurious notifications to unrelated GitHub users.

@greg-1-anderson greg-1-anderson merged commit 5a68dce into master Oct 19, 2017
# Relative aliases are always taken from the Drupal root.
# - 'files': Path to 'files' directory. This will be looked up if not
# specified.
# - 'drush-script': Path to the remot Drush command.

Choose a reason for hiding this comment

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

Typo. :s/remot/remote/.

@geek-merlin
Copy link
Contributor

FTR from #5675:

My one thought is to pause and ask if we really need this alias-parameters feature.

For some use cases, default command options defined by aliases can NOT be replaced by default command options defined by drush config, because in a deployment use case, there is NO drush config before the first deployment.

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

Successfully merging this pull request may close these issues.

6 participants