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

Add key and verifyChain connection options #40

Merged

Conversation

mitch-roblox
Copy link
Contributor

This adds options for key and verifyChain to the per-server settings.

Copy link
Contributor

@martinisoft martinisoft left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@mitch-roblox
Copy link
Contributor Author

Hi, just checking to see if this can be merged?

@martinisoft
Copy link
Contributor

@mitch-roblox right now you have two conflicts so you'll have to rebase against master for us to consider merging this.

@mitch-roblox
Copy link
Contributor Author

Hi, I rebased this against master and now it's passing the tests. Please let me know if there is anything else you need from me. Thanks!

@mitch-roblox
Copy link
Contributor Author

@martinisoft Hi, just checking in on this. Thanks!

Copy link
Contributor

@martinisoft martinisoft left a comment

Choose a reason for hiding this comment

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

Little nitpick to correct then I am OK with a merge.

@@ -99,4 +99,7 @@ TIMEOUTclose = <%= opts['timeout_close'] ? 1 : 0 %>
<% unless opts['client'].nil? -%>
client = <%= opts['client'] ? "yes" : "no" %>
<% end -%>
<% unless opts['verify_chain'].nil? %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Little nitpick, but I'd like you to add the whitespace chomp at the end of the tag like the other unless blocks for consistency. At the end change %> to -%> on this line and the end line as well.

@mitch-roblox
Copy link
Contributor Author

@martinisoft Ah, good catch, thank you! I've pushed up the requested fix.

@mitch-roblox
Copy link
Contributor Author

@martinisoft Hello, just checking in on this. Thank you!

@martinisoft
Copy link
Contributor

I'll work on merging and shipping this later today. Have a bit of a backlog to work through first. Thank you again for your contribution. ❤️

@mitch-roblox
Copy link
Contributor Author

Hello, I hate to be a bother, but just checking in on this. Thank you!

@mitch-roblox
Copy link
Contributor Author

@martinisoft Hi, just checking in on this. I would greatly appreciate it if it could be merged soon. Thank you!

@martinisoft
Copy link
Contributor

Thanks for checking in @mitch-roblox I'll get to a release shortly. On conference wifi so we'll see how well this works.

@martinisoft martinisoft merged commit 993cec5 into sous-chefs:master Nov 8, 2017
@martinisoft
Copy link
Contributor

All merged and shipped. It should be available on the supermarket shortly. 👍

@lock
Copy link

lock bot commented Dec 19, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 19, 2018
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.

None yet

3 participants