InABox: Add support for /region alias for /datacentre #236
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lerlacher-fm
force-pushed
the
le-box-create-region
branch
from
b16fc6e
to
564a872
Compare
wolfsage
reviewed
Jun 27, 2024
lib/Synergy/Reactor/InABox.pm
Outdated
| @@ -159,15 +159,23 @@ command box => { | |||
|
|
|||
| my %switches = map { my ($k, @rest) = @$_; $k => \@rest } @$switches; | |||
|
|
|||
| if (!exists $switches{datacentre}) { | |||
There was a problem hiding this comment.
I think the end goal here is that datacentre, datacenter, and region all mean the same thing, and providing more than 1 of them is a mistake. I think we can simplify this greatly with:
my @got = grep { exists $switches{$_} } qw(datacenter datacentre region);
if (@got > 1) {
die "The following options are mutually exclusive: @got\n";
}
$switches{datacentre} = delete $switches{$got[0]};
Thoughts?
There was a problem hiding this comment.
Yes, that looks much cleaner! Let me test if this works in all cases.
There was a problem hiding this comment.
I have tested this and slightly adjusted it to make it all work nicely.
rjbs
force-pushed
the
main
branch
2 times, most recently
from
542b6ec
to
091bf32
Compare
lerlacher-fm
force-pushed
the
le-box-create-region
branch
3 times, most recently
from
d84d769
to
33e8ab1
Compare
rjbs
requested changes
Jul 1, 2024
rjbs
force-pushed
the
main
branch
3 times, most recently
from
747f6ee
to
10212ce
Compare
I always type "/region" on the first try because I've spent too much time in the cloud so region is the natural concept in my head rather than datacentre. So let's make the robot accomodate the human.
lerlacher-fm
force-pushed
the
le-box-create-region
branch
from
33e8ab1
to
c18948f
Compare
|
@rjbs I have applied your suggestions, rebased, and gave it a very rudimentary test-drive. |
|
@lerlacher-fm Thanks for humoring me, change looks good (and did indeed drop some complexity imho). 👍🏽 |
rjbs
approved these changes
Jul 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I always type "/region" on the first try because I've spent too much time in the cloud so region is the natural concept in my head rather than datacentre. So let's make the robot accomodate the human.