Skip to content
This repository was archived by the owner on Feb 27, 2020. It is now read-only.

[Reviewer: Ellie] Multiple domain support#485

Merged
mirw merged 4 commits intodevfrom
sto714
Apr 6, 2014
Merged

[Reviewer: Ellie] Multiple domain support#485
mirw merged 4 commits intodevfrom
sto714

Conversation

@mirw
Copy link
Contributor

@mirw mirw commented Apr 4, 2014

Ellie,

Please can you review my changes for multiple domain support?

Thanks,

Matt

Copy link
Contributor

Choose a reason for hiding this comment

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

There's 4 slightly different ways here of adding a parameter to the DAEMON_ARGs. Can you tidy this up as part of this fix so that they match? Also, I assume there isn't a problem here if $additional_home_domains doesn't exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's fine if additional_home_domains doesn't exist - it's just blank and fails the test.

Let's discuss on Monday about commonizing the ways of adding a parameter to DAEMON_ARGS. I think the if ... then ... fi vs [ ! ... ] || ... distinction makes sense - the former is better for more complex expressions and the latter for simpler. The ..._arg approach vs directly manipulating DAEMON_ARGS is more interesting.

@eleanor-merry
Copy link
Contributor

Couple of minor questions, otherwise fine

@eleanor-merry eleanor-merry assigned mirw and unassigned eleanor-merry Apr 4, 2014
@mirw mirw merged commit 55701d0 into dev Apr 6, 2014
@mirw mirw deleted the sto714 branch April 6, 2014 13:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants