consolidation / site-alias

Manage alias records for local and remote sites.
Other
59 stars 15 forks source link

Favor requested data over @self alias #42

Closed wongjn closed 3 years ago

wongjn commented 3 years ago

Overview

This pull request:

Q A
Bug fix? yes
New feature? no
Has tests? yes
BC breaks? no
Deprecations? no

Summary

When using drush site:alias @my.alias, it currently uses @self.alias (if it exists) instead of @my.alias since site-alias will use the same named environment in @self if it exists instead of the requested environment in @my. This also has implications for other commands that use site aliases as parameters, such as sql:sync. This PR ensures that the specified alias environment data takes precedence over any @self environment data with the same name.

Description

Might be considered BC break if anyone is using the incorrect behavior. Resolves drush-ops/drush#4424.

wongjn commented 3 years ago

FYI: test failures are unrelated to this change and were failing before changes in this PR.

greg-1-anderson commented 3 years ago

This looks like a straight-up bugfix to me. Anyone who expects the inherited value to take precedence should delete the overriding value from the more specific alias, IMO, although there may be some use-case that I am not considering, e.g. a value that might be used in multiple contexts, only some of which are overridden. I can't think of a way off the top of my head to get in a situation where the BC break would be significant. I'm inclined to include this fix in the current release.

@weitzman what do you think?

greg-1-anderson commented 3 years ago

Failing test is just a case of the list of actual results being in a different order than the list of expected results, sometimes. Just needs a sort; unrelated to this PR.

greg-1-anderson commented 3 years ago

Released 3.1.1.