Skip to content

Conversation

@greg-1-anderson
Copy link
Member

Overview

This pull request:

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

Summary

Adds the ability to specify locations for alias files, e.g. @isp.site.env will look for the alias site.env, but only in alias files where the immediate parent is named "isp".

Motivation

In Drush 8 and earlier, complex aliases such as @foo.bar.baz.boz were possible. This scheme was too flexible, though; the previous example could have referred to the site bar.baz in the group foo, or it could have been the size baz in the group foo.bar. The only way to evaluate such expressions was to evaluate all aliased and then search for matches.

Drush 9 greatly simplified alias parsing for improved performance. For a while, alias group files were supported (@group.site.env), so long as none of the component parts (the group, site or env) contained a .; however, group files were removed so that all alias files could follow the same format. (Group files added another level of nesting to define the groups.)

The elimination of this feature made the code much smaller and more comprehensible; however, it left a functionality gap. While it is possible to define a new directory to store all of the aliases for a single isp or remote server (or any other grouping of aliases), there is no way to specify which group an alias belongs to, or to list all of the aliases in a given group.

This PR re-introduces alias groups, but with a much simpler implementation. Now, an alias group is simply a location filter that names the directory that site aliases in the group are stored in.

@coveralls
Copy link

coveralls commented Jul 3, 2018

Pull Request Test Coverage Report for Build 23

  • 57 of 72 (79.17%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+4.5%) to 54.957%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/SiteAliasFileDiscovery.php 12 13 92.31%
src/SiteAliasName.php 21 26 80.77%
src/SiteAliasManager.php 0 9 0.0%
Totals Coverage Status
Change from base Build 17: 4.5%
Covered Lines: 255
Relevant Lines: 464

💛 - Coveralls

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.

Are any changes/impacts needed for drush's site-alias command? also note that alias files are listed st command. that doesn't need changes AFAICT.

README.md Outdated
### Alias naming conventions

Site alias names always begin with a `@`, and typically are divided in two parts: the site name, and the environment name, each separated by a dot. Neither a site name not an enviornment name may contain a dot. An example alias that referenced the `dev` envionment of the site `example` might therefore look something like:
Site alias names always begin with a `@`, and typically are divided in three parts: the alias file location (optional), the site name, and the environment name, each separated by a dot. None of these names may contain a dot. An example alias that referenced the `dev` envionment of the site `example` in the `myisp` directory might therefore look something like:
Copy link
Member

Choose a reason for hiding this comment

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

typo: envionment

@example.dev
@myisp.example.dev
```
The location name is optional. If specified, it will only consider alias files located in directories with the same name as the provided location name. The remainder of the path is immaterial; only the directory that is the immediate parent of the site alias file is relevant. The location name may be omitted, e.g.:
Copy link
Member

Choose a reason for hiding this comment

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

unless i'm mistaken, this encourages another level of nesting of aliases. but later in the docs we say that no deep searching is done for alias files. so folks must point explicitly to 'schools' dir?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, with this PR you must explicitly add every new location to your config directory, or the alias will not be found.

We could consider increasing the search depth by one, but for some locations that might be undesirable. We could consider adding a config location /path/to/alias/* to add every directory under a single path. I decided to start with the requirement to add each location explicitly. As you said elsewhere, this is sort of a "Drush for ISPs" feature. Users should only have a handful of ISPs, so unless this feature starts getting used for other purposes, I think the requirement for explicit config is acceptable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also note that extra locations may be either inside of sites or next to it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, the site-alias project allows deep searching -- it has an API to set the depth of the search. The search depth defaults to current-directory only (no deep searching), and Drush does not set it to allow deeper searching.

@greg-1-anderson
Copy link
Member Author

This PR did not require any changes to the API, and therefore required no changes in Drush for the site-alias command or the st command.

@greg-1-anderson greg-1-anderson merged commit 5ec014c into master Jul 5, 2018
manarth pushed a commit to manarth/site-alias that referenced this pull request Sep 14, 2022
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.

4 participants