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.

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

Fix typos in metrics doc #662

Closed
wants to merge 1 commit into from
Closed

Conversation

avlitman
Copy link

@avlitman avlitman commented May 12, 2024

metrics doc contain names with double underscore.

Fixes #...

Changes proposed

  • names will be correct.

Checklist

  • Includes tests covering the new functionality?
  • Ready for review
  • Follows CONTRIBUTING rules

Notify

@Waschndolos

metrics doc contain names with double underscore.
Signed-off-by: avlitman <alitman@redhat.com>
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Markdownlint (reported by Codacy) found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

| default_jenkins_builds<buildname>_last_build_start_time_milliseconds | Last build start timestamp in milliseconds | gauge |
| default_jenkins_builds<buildname>_last_build_tests_total | Number of total tests during the last build | gauge |
| default_jenkins_builds<buildname>_last_last_build_tests_skipped | Number of skipped tests during the last build | gauge |
| default_jenkins_builds<buildname>_last_build_tests_failing | Number of failing tests during the last build | gauge |

Choose a reason for hiding this comment

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

The Plugin outputs like default_jenkins_builds_last_build_tests_failing so the _ seems right or am I missing something here?

Copy link
Author

Choose a reason for hiding this comment

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

Copy link

@Waschndolos Waschndolos left a comment

Choose a reason for hiding this comment

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

Not sure why the _ has been removed here. Looks right to me when I check the endpoint. Like for example:

default_jenkins_builds_last_build_result_ordinal or default_jenkins_builds_last_build_result

@avlitman avlitman requested a review from Waschndolos May 20, 2024 10:17
@avlitman
Copy link
Author

avlitman commented May 20, 2024

@Waschndolos Oh I see you changes it here 304ce25 after I opened this pr..

BTW i still think default_jenkins_builds_<buildname>_last* is not correct in my case, it's only default_jenkins_builds_last* for my endpoint.

@Waschndolos
Copy link

Thanks for reaching back. I'll need to test this again - maybe I'm overseeing something here

@Waschndolos
Copy link

I'll go ahead and close this PR and fix the documentation in #666 . Guess it's too confusing right now

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.

None yet

2 participants