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 guidelines on metrics and log messages #5270

Merged
merged 8 commits into from
Dec 11, 2023
Merged

add guidelines on metrics and log messages #5270

merged 8 commits into from
Dec 11, 2023

Conversation

wonko
Copy link
Contributor

@wonko wonko commented Dec 8, 2023

Added a personal view on metrics and log messages as requested in #5259 (comment)

@wonko wonko requested a review from a team as a code owner December 8, 2023 14:20
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 8, 2023

Thank you for your contribution! 🙏 We will review your PR as soon as possible.

While you are waiting, make sure to:

Learn more about:

@wonko
Copy link
Contributor Author

wonko commented Dec 8, 2023

@JorTurFer @tomkerkhove @zroubalik As requested, very biased view on metrics and log messages.

@wonko wonko mentioned this pull request Dec 8, 2023
Merged
4 tasks
tomkerkhove
tomkerkhove approved these changes Dec 11, 2023
View reviewed changes
Copy link
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

This guidance looks good to me to start with, added some formatting/suggestions

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@wonko @tomkerkhove
Apply suggestions from code review
5a045a9 
Co-authored-by: Tom Kerkhove <[email protected]>
Signed-off-by: Bernard Grymonpon <[email protected]>
zroubalik
zroubalik reviewed Dec 11, 2023
View reviewed changes
Copy link
Member

@zroubalik zroubalik 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.

Thanks a lot, I added a minor nits to the logging section

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
wonko and others added 2 commits December 11, 2023 12:15
@wonko @zroubalik
Apply suggestions from code review
0eb3821 
Co-authored-by: Zbynek Roubalik <[email protected]>
Signed-off-by: Bernard Grymonpon <[email protected]>
@wonko @zroubalik
Apply suggestions from code review
8875b15 
Co-authored-by: Zbynek Roubalik <[email protected]>
Signed-off-by: Bernard Grymonpon <[email protected]>
wonko
wonko commented Dec 11, 2023
View reviewed changes
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
zroubalik and others added 2 commits December 11, 2023 13:05
@zroubalik @wonko
Update CONTRIBUTING.md
db75bfb 
Co-authored-by: Bernard Grymonpon <[email protected]>
Signed-off-by: Zbynek Roubalik <[email protected]>
@zroubalik @wonko
Update CONTRIBUTING.md
20a6167 
Co-authored-by: Bernard Grymonpon <[email protected]>
Signed-off-by: Zbynek Roubalik <[email protected]>
zroubalik
zroubalik approved these changes Dec 11, 2023
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot!

@zroubalik zroubalik enabled auto-merge (squash) December 11, 2023 12:08
zroubalik
zroubalik reviewed Dec 11, 2023
View reviewed changes
CONTRIBUTING.md Outdated Show resolved Hide resolved
@zroubalik
Update CONTRIBUTING.md
8495afe 
Signed-off-by: Zbynek Roubalik <[email protected]>
zroubalik
zroubalik reviewed Dec 11, 2023
View reviewed changes
Copy link
Member

@zroubalik zroubalik 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.

@wonko could you please regenerate Table of contents for this file? We use doctoc: https://github.com/thlorenz/doctoc#usage-as-a-git-hook

You can run this command to do generate TOC without any installation:

docker run --rm -v "$(pwd)":/app peterdavehello/npm-doctoc doctoc CONTRIBUTING.md

@wonko
TOC updated
b5f9834 
Signed-off-by: Bernard Grymonpon <[email protected]>
auto-merge was automatically disabled December 11, 2023 15:02

Head branch was pushed to by a user without write access

@zroubalik zroubalik merged commit 646bb6b into kedacore:main Dec 11, 2023
19 checks passed
@tppalani
Copy link

Hi All

I have already installed Keda inside my eks cluster now I want use above future which will show quote recommendations this will very useful development team can define limit and request according to the deployment yaml

can you tell me how to install the new future in my existing cluster

Keda version 2.10.1

toniiiik pushed a commit to toniiiik/keda that referenced this pull request Jan 15, 2024
@wonko @tomkerkhove
@zroubalik @toniiiik
add guidelines on metrics and log messages (kedacore#5270)
6008c0e 
Signed-off-by: Bernard Grymonpon <[email protected]>
Signed-off-by: Zbynek Roubalik <[email protected]>
Co-authored-by: Tom Kerkhove <[email protected]>
Co-authored-by: Zbynek Roubalik <[email protected]>
Signed-off-by: anton.lysina <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zroubalik zroubalik zroubalik approved these changes

@tomkerkhove tomkerkhove tomkerkhove approved these changes

Assignees
No one assigned
Labels
skip-e2e
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wonko @tppalani @zroubalik @tomkerkhove