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.

Sign up for GitHub

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

Jump to bottom

Add SSL Breaking changes for logstash-output-http to breaking changes… #16701

Merged
merged 4 commits into from
Nov 26, 2024
Merged

Add SSL Breaking changes for logstash-output-http to breaking changes… #16701

merged 4 commits into from
Nov 26, 2024

Conversation

robbavey
Copy link
Member

@robbavey robbavey commented Nov 20, 2024 •
edited by karenzone
Loading

@robbavey robbavey requested a review from karenzone November 20, 2024 16:14
@donoghuc
Copy link
Member

I like the idea here. For all the other plugins we will continue with sections that have the plugin name and the obsolete parameters. Looks like the docs build is red. The only thing I saw in the logs is a warning about use of headers. Not sure if that is the cause. Once we get this base example sorted out we can start making sure the other plugins get a similar treatment :)

karenzone
karenzone reviewed Nov 21, 2024
View reviewed changes
Copy link
Contributor

@karenzone karenzone left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Left suggestions and broader guidelines inline.
Let's discuss and iterate. When we agree, let's apply these style guidelines for the other 10 plugins with changes to SSL settings.

docs/static/breaking-changes-90.asciidoc Outdated Show resolved Hide resolved
@robbavey robbavey marked this pull request as ready for review November 21, 2024 20:58
@robbavey
Copy link
Member Author

Doc Preview:

https://logstash_bk_16701.docs-preview.app.elstc.co/

@karenzone karenzone mentioned this pull request Nov 22, 2024
Closed
@karenzone
Copy link
Contributor

Follow up:

Now that I've played with it, I still like the collapsible content, but I don't like the interaction details in my suggestion.
The down arrow and the link to plugin docs are too close together, and make for an ambiguous information experience. Restating the obvious, if you click the down arrow, the section expands to show deleted settings. If you click the plugin text, you jump to the plugin docs. I'll tweak my suggestion accordingly.

Also, the dropdown arrow is a common UI element and we use it extensively in our docs. I'll put in a note to explain, and we can use it--or not.

Screenshot 2024-11-22 at 3 28 22 PM

@karenzone karenzone mentioned this pull request Nov 25, 2024
Open
3 tasks
@github-actions GitHub Actions
Copy link
Contributor

📃 DOCS PREVIEW ✨ https://logstash_bk_16701.docs-preview.app.elstc.co/diff

karenzone
karenzone approved these changes Nov 26, 2024
View reviewed changes
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!

@robbavey robbavey merged commit ccde1eb into elastic:main Nov 26, 2024
5 checks passed
donoghuc added a commit to donoghuc/logstash that referenced this pull request Dec 2, 2024
@donoghuc
Add breaking changes docs for input-elasticsearch
d3f1645 
This commit follows the pattern established in
elastic#16701 for indicating obsolete ssl
settings in logstash core plugins.
@donoghuc donoghuc mentioned this pull request Dec 2, 2024
Merged
donoghuc added a commit to donoghuc/logstash that referenced this pull request Dec 3, 2024
@donoghuc
Add breaking changes docs for input-elasticsearch
f54f9ff 
This commit follows the pattern established in
elastic#16701 for indicating obsolete ssl
settings in logstash core plugins.
donoghuc added a commit that referenced this pull request Dec 9, 2024
@donoghuc
Add breaking changes docs for input-elasticsearch (#16744)
095fbbb 
This commit follows the pattern established in
#16701 for indicating obsolete ssl
settings in logstash core plugins.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karenzone karenzone karenzone approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@robbavey @donoghuc @karenzone @jsvd