-
Notifications
You must be signed in to change notification settings - Fork 396
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
Fixes #1087: Auto-discovery of multisite.name. #1119
Fixes #1087: Auto-discovery of multisite.name. #1119
Conversation
I'm getting some slightly unexpected output here. It looks like the ACSF task adds an additional site. Should the expected value then be |
@bobbygryzynger yeah that's right, acsf does add a "g' site. |
No problem, this is passing now on travis with |
@@ -5,6 +5,7 @@ | |||
<php> | |||
<const name="BLT_ENV" value="ci"/> | |||
<const name="BLT_ALIAS" value="self"/> | |||
<const name="BLT_MULTISITE_NAME" value="default,g"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this defined as a constant in the phpunit.xml rather than just hardcoded in the test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it would be better, should this value change, that the test itself wouldn't need to be updated. Also, implementing projects would have an easier time overriding this should they wish to run the tests against their own configuration.
If you'd rather this be hardcoded, I'm fine with making that change, just let me know.
Woot! |
Fixes #1087.
Changes proposed:
multisite.name
list automatically if not provided by user.