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

[SW-1708] Simplify distribution of security files #1614

Merged
merged 3 commits into from Nov 13, 2019
Merged

Conversation

jakubhava
Copy link
Contributor

@jakubhava jakubhava commented Nov 12, 2019

This proposal introduce a small cleanup. I thing we can keep one code path for all the cases using this solution.

What do you think? I have missed this during the review.

Maybe there is a reason for doing it like now, I just not see it :)

@mn-mikke
Copy link
Collaborator

@jakubhava I think this is a great improvement! It seems I got lost in various scenarios. In that case I think you can remove the third parameter from property specification. E.g: https://github.com/h2oai/sparkling-water/pull/1614/files#diff-ed060ffa1e96e1fe4dc30499d7cc9a54L263

@jakubhava
Copy link
Contributor Author

Thanks @mn-mikke! I started playing with SSL communication and got this idea.

@jakubhava jakubhava merged commit 994c99c into master Nov 13, 2019
@jakubhava jakubhava deleted the jh/sw-1708 branch November 13, 2019 12:45
jakubhava added a commit that referenced this pull request Nov 13, 2019
jakubhava added a commit that referenced this pull request Nov 13, 2019
@mn-mikke
Copy link
Collaborator

Sorry, I should have mentioned that after the third item from the property spec. The return time of this needs to be modified.

@jakubhava
Copy link
Contributor Author

Ah, good catch, let me fix it

jakubhava added a commit that referenced this pull request Nov 13, 2019
jakubhava added a commit that referenced this pull request Nov 13, 2019
@jakubhava
Copy link
Contributor Author

Fixed, sorry, my bad

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants