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

Improve CC documentation about CPU capacity and utilization #5992

Merged
merged 9 commits into from Dec 27, 2021
Merged

Improve CC documentation about CPU capacity and utilization #5992

merged 9 commits into from Dec 27, 2021

Conversation

fvaleri
Copy link
Contributor

@fvaleri fvaleri commented Dec 5, 2021

This is to add a couple of important notes from the discussion in #5951.

@fvaleri
Improve CC documentation about CPU capacity and utilization
e07349e 
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
@fvaleri
Copy link
Contributor Author

fvaleri commented Dec 5, 2021

cc @kyguy

@strimzi-ci
Copy link

Can one of the admins verify this patch?

@fvaleri
Clarify Kafka resources recommendation.
0665c84 
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
@fvaleri
Copy link
Contributor Author

fvaleri commented Dec 5, 2021

Did some changes based on your feedback. Thanks.

@scholzj scholzj added this to the 0.27.0 milestone Dec 5, 2021
@scholzj
Copy link
Member

scholzj commented Dec 5, 2021

PS: It seems the build is failing because it does not like the e.g. - for example should be used instead.

@fvaleri
Fix formatting error
ca3265a 
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
PaulRMellor
PaulRMellor reviewed Dec 7, 2021
View reviewed changes
Copy link
Contributor

@PaulRMellor PaulRMellor left a comment

Choose a reason for hiding this comment

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

Hi Federico. Nice addition. I've made some suggestions. I found some of the sentences hard to read, so I've tried to simplify.

documentation/modules/cruise-control/proc-generating-optimization-proposals.adoc Outdated Show resolved Hide resolved
documentation/modules/cruise-control/proc-generating-optimization-proposals.adoc Outdated Show resolved Hide resolved
documentation/modules/cruise-control/proc-generating-optimization-proposals.adoc Outdated Show resolved Hide resolved
documentation/modules/cruise-control/ref-cruise-control-configuration.adoc Outdated Show resolved Hide resolved
@fvaleri
Copy link
Contributor Author

fvaleri commented Dec 7, 2021

@PaulRMellor Thanks for your suggestions.

I rephrased a bit some of them, because the meaning was not exactly the same after your changes. I hope it still reads well.

kyguy
kyguy reviewed Dec 7, 2021
View reviewed changes
Copy link
Member

@kyguy kyguy left a comment

Choose a reason for hiding this comment

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

Hey @fvaleri thanks for the PR!

I left a couple of notes, let me know what you think!

One other thing, from what I remember we enforce having one sentence per line (to make it easier to leave feedback in the PR review phase)

documentation/modules/cruise-control/proc-generating-optimization-proposals.adoc Outdated Show resolved Hide resolved
documentation/modules/cruise-control/proc-generating-optimization-proposals.adoc Outdated Show resolved Hide resolved
documentation/modules/cruise-control/ref-cruise-control-configuration.adoc Outdated Show resolved Hide resolved
@fvaleri
Copy link
Contributor Author

fvaleri commented Dec 7, 2021

Thanks @PaulRMellor and @kyguy for your feedback. I did some changes based on that.

PaulRMellor
PaulRMellor approved these changes Dec 9, 2021
View reviewed changes
Copy link
Contributor

@PaulRMellor PaulRMellor left a comment

Choose a reason for hiding this comment

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

👍 Couple of small suggestions.

@scholzj scholzj modified the milestones: 0.27.0, 0.28.0 Dec 18, 2021
@fvaleri
Improve readability.
a2920d2 
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
@fvaleri
Fix memory recommendation
e358e36 
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
@fvaleri
Improve note about accurate RebalanceProposals
0b01b75 
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
@fvaleri
Copy link
Contributor Author

fvaleri commented Dec 23, 2021

@kyguy @scholzj I think the last commit reflects our great discussion. I'm open to further improvements or clarifications.

@fvaleri
Improve formatting
e5625f7 
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
@fvaleri
One sentence per line everywhere
572a0ec 
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
@fvaleri
Clarify excludedTopics with regards to utilization calculation
d4f5dad 
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
kyguy
kyguy approved these changes Dec 27, 2021
View reviewed changes
Copy link
Member

@kyguy kyguy left a comment

Choose a reason for hiding this comment

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

Looks great @fvaleri!

@scholzj
Copy link
Member

scholzj commented Dec 27, 2021

Thanks for the PR @fvaleri

@scholzj scholzj merged commit cfcd49c into strimzi:main Dec 27, 2021
@fvaleri fvaleri deleted the cc-doc branch December 28, 2021 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scholzj scholzj scholzj approved these changes

@kyguy kyguy kyguy approved these changes

@PaulRMellor PaulRMellor PaulRMellor approved these changes

@ppatierno ppatierno Awaiting requested review from ppatierno

@tomncooper tomncooper Awaiting requested review from tomncooper

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

Successfully merging this pull request may close these issues.

None yet
5 participants
@fvaleri @strimzi-ci @scholzj @kyguy @PaulRMellor