Skip to content

Conversation

@donoghuc
Copy link
Contributor

Remove the deprecated ssl config option and document how to replace it with ssl_enabled. Prep the 2.0.0 release.

This commit marks the ssl configuration as obsolete. The docs have been updated
as well as the tests. This change will be rolled out in a 2.0.0 release which
has also been prepped in this work.
@donoghuc donoghuc force-pushed the GH-9-obsolete-ssl-settings branch from a4da16c to e514cfe Compare December 18, 2024 22:30
@donoghuc donoghuc requested a review from robbavey December 18, 2024 22:37
Copy link
Member

@robbavey robbavey left a comment

Choose a reason for hiding this comment

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

LGTM - Over to @karenzone for doc review

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.

On or around line 161, add note/link wrt deprecated settings that have been removed:
"NOTE: As of version 2.0.0 of this plugin, a previously deprecated SSL setting has been removed.
Please check out <<plugins-{type}s-{plugin}-obsolete-options>> for details."


I tweaked the content a bit from the standard to make more sense with only one setting deleted.

ALSO, please check out inline suggestion.

@donoghuc donoghuc force-pushed the GH-9-obsolete-ssl-settings branch from 451ac1f to e514cfe Compare December 23, 2024 18:26
@karenzone
Copy link
Contributor

Please also pick up initial suggestion noted in: #11 (review)

GitHub doesn't allow suggestions to sections without changes. They're easy to miss.

Add a note on where to look for obsolete options, fix whitespace.
@donoghuc
Copy link
Contributor Author

Sorry, yeah i had an isssue where i committed your suggestion, then when i tried to pull it down locally i ended up on a different local index. I thought i had force pushed it but it was the wrong source<->dest. I think i've got it cleaned up now. Monday problems 😅

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

@donoghuc donoghuc merged commit 3da4b9b into logstash-plugins:main Dec 23, 2024
2 checks passed
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