Skip to content

added executor_threads documentation#421

Merged
andsel merged 4 commits into
logstash-plugins:masterfrom
111andre111:doc-threads
Aug 31, 2021
Merged

added executor_threads documentation#421
andsel merged 4 commits into
logstash-plugins:masterfrom
111andre111:doc-threads

Conversation

@111andre111

Copy link
Copy Markdown
Contributor

Thanks for contributing to Logstash! If you haven't already signed our CLA, here's a handy link: https://www.elastic.co/contributor-agreement/

@111andre111

Copy link
Copy Markdown
Contributor Author

Here was a parallel one:
#409

@andsel andsel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just left a minimal suggestion

Comment thread docs/index.asciidoc Outdated
@andsel andsel requested a review from karenzone July 28, 2021 06:59
@111andre111

Copy link
Copy Markdown
Contributor Author

@andsel
What is the influcence of the draft PR #410
mentioned in #409?

@andsel

andsel commented Jul 28, 2021

Copy link
Copy Markdown
Contributor

that description is more extended, we could use that, and close that draft PR with this one

@andsel

andsel commented Jul 28, 2021

Copy link
Copy Markdown
Contributor

the #410 is some more deep, where the #409 and this are only a face of the problem it's trying to fix

@andsel

andsel commented Jul 29, 2021

Copy link
Copy Markdown
Contributor

Hi @111andre111 I've rebased to master branch fixing conflicts and improved description.

@111andre111

Copy link
Copy Markdown
Contributor Author

So I assume we can merge this one and #409 as well if there weren't duplicates plus I assume #410 still will need some investigation time, correct? @andsel

@andsel

andsel commented Aug 9, 2021

Copy link
Copy Markdown
Contributor

Yes correct @111andre111, but Changelog needs a fix

@111andre111

Copy link
Copy Markdown
Contributor Author

@andsel I have fixed now the Changelog

@karenzone karenzone left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for adding this important documentation. Nice job explaining it. I left some minor suggestions inline for your consideration.

Comment thread docs/index.asciidoc
@andsel andsel self-requested a review August 11, 2021 06:41

@andsel andsel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, merge squashing the commits please, or ping me if you need I do it

@111andre111

Copy link
Copy Markdown
Contributor Author

@andsel Sorry if I didn't manage the squash. I always got an error message.

@andsel

andsel commented Aug 31, 2021

Copy link
Copy Markdown
Contributor

@111andre111 I've rebased to master there were some conflicts on CHANGELOG.md ao probably that prohibited the merge.
I don't publish yet this, I left the version x.y.z so that a next bugfix or feature also publish this.

@111andre111

Copy link
Copy Markdown
Contributor Author

Ah I see. I didn't see that. Thank you @andsel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants