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

Mark deprecated SSL settings as obsolete #228

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

robbavey
Copy link
Contributor

@robbavey robbavey commented Dec 2, 2024

This commit marks the following SSL settings as obsolete:

ssl_cert, which should be replaced by ssl_certificate
ssl_enable, which should be replaced by ssl_enabled
ssl_verify, which should be replaced by ssl_client_authentication when mode is server or ssl_verification_modewhen mode is client

Relates: #225

This commit marks the following SSL settings as obsolete:

`ssl_cert`, which should be replaced by `ssl_certificate`
`ssl_enable`, which should be replaced by `ssl_enabled`
`ssl_verify`, which should be replaced by `ssl_client_authentication` when `mode` is `server` or `ssl_verification_mode`when mode is `client`
CHANGELOG.md Outdated Show resolved Hide resolved
docs/index.asciidoc Outdated Show resolved Hide resolved
Copy link
Contributor

@donoghuc donoghuc left a comment

Choose a reason for hiding this comment

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

Just a couple small docs comments. Code removal looks clean.

Co-authored-by: Cas Donoghue <cas.donoghue@gmail.com>
Copy link
Contributor

@donoghuc donoghuc left a comment

Choose a reason for hiding this comment

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

LGTM

spec/inputs/tcp_spec.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@donoghuc donoghuc left a comment

Choose a reason for hiding this comment

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

Assuming CI is green i think this is good! Apologies for not catching everything on the first take 😬

@robbavey
Copy link
Contributor Author

Assuming CI is green i think this is good! Apologies for not catching everything on the first take 😬

All good! Thanks for being thorough and catching all of my 🤦 ...

@robbavey robbavey requested a review from karenzone December 10, 2024 21:11
@robbavey
Copy link
Contributor Author

Ready for doc review @karenzone

Copy link
Contributor

@karenzone karenzone left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@karenzone karenzone left a comment

Choose a reason for hiding this comment

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

Updated text to align with messaging for other plugins

Copy link
Contributor

@karenzone karenzone left a comment

Choose a reason for hiding this comment

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

Suggestion inline

Comment on lines +342 to +343
WARNING: As of version `7.0.0` of this plugin, the following configuration options are no longer available,
and have been replaced by the following options:
Copy link
Contributor

@karenzone karenzone Dec 23, 2024

Choose a reason for hiding this comment

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

Suggested change
WARNING: As of version `7.0.0` of this plugin, the following configuration options are no longer available,
and have been replaced by the following options:
WARNING: As of version `7.0.0` of this plugin, some configuration options have been replaced.
The plugin will fail to start if it contains any of these obsolete options.

@karenzone karenzone self-requested a review December 23, 2024 18:22
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