Skip to content

api: add helper Dialer implementations #371

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

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

DerekBum
Copy link

@DerekBum DerekBum commented Jan 18, 2024

To disable SSL by default we want to transfer OpenSslDialer to the go-openssl repository. In order to do so, we need to minimize the amount of copy-paste of the private functions.

AuthDialer is created as a dialer-wrapper, that calls authentication methods.
ProtoDialer is created to check the ProtocolInfo in the created connection.
GreetingDialer is created to fill the Greeting in the created connection.

I didn't forget about (remove if it is not applicable):

Related issues:
Part of #301

@DerekBum DerekBum force-pushed the DerekBum/gh-301-disable-ssl-by-default branch 2 times, most recently from 8ce3479 to 582353f Compare January 29, 2024 12:28
@DerekBum DerekBum changed the title api: disable SSL by default api: create AuthDialer and ProtocolDialer Jan 29, 2024
@DerekBum DerekBum force-pushed the DerekBum/gh-301-disable-ssl-by-default branch from 582353f to 9292448 Compare January 29, 2024 12:30
@DerekBum DerekBum marked this pull request as ready for review January 29, 2024 12:49
@DerekBum DerekBum force-pushed the DerekBum/gh-301-disable-ssl-by-default branch 3 times, most recently from 0f7d508 to 7038359 Compare January 30, 2024 13:15
@DerekBum DerekBum force-pushed the DerekBum/gh-301-disable-ssl-by-default branch 3 times, most recently from 1a49b28 to 963c147 Compare January 31, 2024 00:49
@DerekBum DerekBum force-pushed the DerekBum/gh-301-disable-ssl-by-default branch from 963c147 to 38edd2a Compare February 1, 2024 09:26
Copy link
Collaborator

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

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

Thank you for the patch!

@oleg-jukovec
Copy link
Collaborator

Please, update the commit message: add GreetingDialer too.

@oleg-jukovec oleg-jukovec requested a review from askalt February 1, 2024 11:09
@DerekBum DerekBum force-pushed the DerekBum/gh-301-disable-ssl-by-default branch from 38edd2a to 023817d Compare February 1, 2024 11:36
@DerekBum
Copy link
Author

DerekBum commented Feb 1, 2024

Please, update the commit message: add GreetingDialer too.

It is in the commit message:

GreetingDialer is created to fill the Greeting in the created connection.

@oleg-jukovec
Copy link
Collaborator

Please, update the commit message: add GreetingDialer too.

It is in the commit message:

GreetingDialer is created to fill the Greeting in the created connection.

But the title is confusing. It could be something like: api: add helper Dialer implementation or something like that.

Copy link

@askalt askalt left a comment

Choose a reason for hiding this comment

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

Thank you for the patch! lgtm

@DerekBum DerekBum changed the title api: create AuthDialer and ProtocolDialer api: add helper Dialer implementations Feb 2, 2024
To disable SSL by default we want to transfer
`OpenSslDialer` to the go-openssl repository.
In order to do so, we need to minimize the amount of
copy-paste of the private functions.

`AuthDialer` is created as a dialer-wrapper, that
calls authentication methods.
`ProtoDialer` is created to receive and check the `ProtocolInfo`
in the created connection.
`GreetingDialer` is created to fill the `Greeting`
in the created connection.

Part of #301
@DerekBum DerekBum force-pushed the DerekBum/gh-301-disable-ssl-by-default branch from 023817d to c56f978 Compare February 2, 2024 13:51
@DerekBum
Copy link
Author

DerekBum commented Feb 2, 2024

Please, update the commit message: add GreetingDialer too.

It is in the commit message:

GreetingDialer is created to fill the Greeting in the created connection.

But the title is confusing. It could be something like: api: add helper Dialer implementation or something like that.

Oh, I thought you were talking about commit body, not header. Updated commit header and PR header too.

@oleg-jukovec oleg-jukovec merged commit e765b0a into master Feb 2, 2024
@oleg-jukovec oleg-jukovec deleted the DerekBum/gh-301-disable-ssl-by-default branch February 2, 2024 15:31
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.

3 participants