search

heroku / heroku-connect-plugin

CLI plugin for Heroku Connect (experimental)
Apache License 2.0
23 stars 8 forks source link

Automatically detect region via the app #66

Closed gulopine closed 7 years ago

gulopine commented 7 years ago

The --region flag still reigns supreme, if supplied, but the default now fetches the region from the app instead. Better yet, it only fetches from the app if the --region flag isn't supplied. So this should allow 99% of users to use the CLI without the --region flag, while still giving a backdoor for those who need it.

Using the Heroku API required making determineRegion asynchronous, which in turn requires a different way to call it (in addition to needing to pass in the Heroku API client). So there are a lot of files changed, but most of them are just a simple usage change.

I also reorganized determineRegions a bit and simplified it. Checking for --region first not only saves us an API call in some isolated cases, it also makes the if read as a positive, rather than a negative, which I find helps the else make a bit more sense. I also changed it to just throw an Error rather than using cli.exit(), since cli.command already catches errors and does the right thing with them.

alouie-sfdc commented 7 years ago

Looks really good. I just have the one question about including dublin.

gulopine commented 7 years ago

@alouie-sfdc I've added Dublin, good catch. Once I get a 👍 here, can you see if you agree thta #64 is ready to merge as well? Then we'll be able to do a release tomorrow.

alouie-sfdc commented 7 years ago

👍 -- both PRs look ready to me. Let's also check in with @sigmavirus24 to get his feedback in the morning.

sigmavirus24 commented 7 years ago

👍 :shipit: