Skip to content
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

Update both heroku to take randomly names #81

Merged
merged 3 commits into from
Mar 14, 2019
Merged

Conversation

onlyhavecans
Copy link
Member

No description provided.

Signed-off-by: Amelia Aronsohn <WagThatTail@Me.com>
@weppos
Copy link
Member

weppos commented Feb 25, 2019

I added @sbastn as reviewer as he's been taking care of several services.

@sbastn do you think we should let the customer enter the whole Heroku host (e.g. antonym-rivalry-b163r7kvcv32we2uaw69gd8a.herokudns.com) or just the random part (e.g. antonym-rivalry-b163r7kvcv32we2uaw69gd8a)?

@weppos
Copy link
Member

weppos commented Feb 25, 2019

@onlyhavecans do you have confirmation this change affects both SSL and non-SSL?

@onlyhavecans
Copy link
Member Author

do you have confirmed this change affects both SSL and non-SSL?

No, and their support site does not help. Do we have a contact or test we can set up?

@weppos
Copy link
Member

weppos commented Feb 26, 2019

Do we have a contact or test we can set up?

You can probably setup a test app in our Heroku account, add a domain without SSL and see what they suggest to use as target hostname.

@onlyhavecans
Copy link
Member Author

Building a test app with no SSL I verified the interface is now unified. If @dnsimple/developers can verify my test is accurate we can likely simplify this down to a single 1-click

screen shot 2019-02-26 at 09 39 22

@onlyhavecans onlyhavecans requested review from a team and removed request for aeden, sbastn and jacegu February 26, 2019 17:42
@weppos weppos requested review from sbastn and removed request for a team February 26, 2019 18:38
@weppos
Copy link
Member

weppos commented Feb 26, 2019

we can likely simplify this down to a single 1-click

Yes. I agree. Sadly, we can't currently remove a one-click service. But I believe @sbastn added an option to deprecate an old one.

@sbastn can you please help?

@sbastn
Copy link
Member

sbastn commented Feb 28, 2019

@weppos and @onlyhavecans, I have simplified the setup, so instead of telling the user to manipulate the text to remove "herokudns.com" from it, they can just paste it all in since they have it in their clipboard.

Previously

screen shot 2019-02-28 at 5 20 59 pm

Now

screen shot 2019-02-28 at 5 21 12 pm

@weppos is correct, we cannot delete a one-click service, but you can deprecate it following this section on the README: https://github.com/dnsimple/dnsimple-services/#remove-services

@weppos
Copy link
Member

weppos commented Mar 1, 2019

@sbastn can you please deprecate heroku-ssl?

Signed-off-by: Amelia Aronsohn <WagThatTail@Me.com>
@onlyhavecans
Copy link
Member Author

I deprecated the service per the docs and re-requested review. What is my deployment steps after approval? Can someone work with me to verify the deploy?

@weppos
Copy link
Member

weppos commented Mar 4, 2019

@onlyhavecans the service repo is updated on deploy. So after a merge it's sufficient to deploy the app and the service changes will be loaded in the app.

You will be able to confirm the changes are live by trying to apply the service and the new settings should appear.

Copy link
Member

@sbastn sbastn left a comment

Choose a reason for hiding this comment

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

👏 thanks @onlyhavecans ‼️

@onlyhavecans onlyhavecans merged commit 01ced92 into master Mar 14, 2019
@onlyhavecans onlyhavecans deleted the random_heroku_domain branch March 14, 2019 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants