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

Repeatable error messages for metric documentation checks #28034

Merged
merged 3 commits into from
Sep 29, 2021
Merged

Repeatable error messages for metric documentation checks #28034

merged 3 commits into from
Sep 29, 2021

Conversation

kovyrin
Copy link
Contributor

@kovyrin kovyrin commented Sep 21, 2021

What does this PR do?

Extracted from a WIP PR: #27549

Sort all keys before checking them for documentation to make sure missing documentation error messages are produced in a predictable and repeatable order.

Why is it important?

This is really helpful when someone is trying to fix multiple issues and wants to make sure a specific issue is fixed before moving one to the next one. Otherwise, the errors are thrown in a random order and every time a new message is presented to the developer making it very confusing if the previous one had been resolved or not.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

How to test this PR locally

  1. Remove a couple of metrics from a random metricbeat module (from the fields.yml file)
  2. Run make update for the given module
  3. Run go unit tests for metricbeat (or for your single module) and it will fail in exactly the same way every time you run your tests, showing you your first metric with missing documentation
  4. Fix one field and run make update
  5. Now your unit tests will again fail in a predictable manner, showing you the next metric you need to document.
  6. Repeat until all tests pass

@kovyrin
Sort all keys before checking them for documentation to make sure mis…
feec024 
…sing documentation error messages are produced in a predictable and repeatable order.

This is really helpful when someone is trying to fix multiple issues and wants to make sure a specific issue is fixed before moving one to the next one. Otherwise, the errors are thrown in a random order and every time a new message is presented to the developer making it very confusing if the previous one had been resolved or not.
@kovyrin kovyrin self-assigned this Sep 21, 2021
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Sep 21, 2021
@kovyrin kovyrin requested a review from a team September 21, 2021 19:09
@kovyrin kovyrin added the Team:Elastic-Agent Label for the Agent team label Sep 21, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/agent (Team:Agent)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Sep 21, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Sep 21, 2021 •
edited by jenkins-beats-ci bot
Loading

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-09-22T13:23:03.861+0000

  • Duration: 145 min 31 sec

  • Commit: 46749eb

Test stats 🧪

Test Results
Failed 0
Passed 17711
Skipped 2768
Total 20479

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 17711
Skipped 2768
Total 20479

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test

  • jenkins run the tests

@kovyrin kovyrin mentioned this pull request Sep 21, 2021
Merged
8 tasks
@mergify
Copy link
Contributor

mergify bot commented Sep 22, 2021

This pull request does not have a backport label. Could you fix it @kovyrin? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@mergify mergify bot added the backport-skip Skip notification from the automated backport with mergify label Sep 22, 2021
@kovyrin kovyrin added backport-v7.16.0 Automated backport with mergify backport-v8.0.0 Automated backport with mergify and removed backport-skip Skip notification from the automated backport with mergify labels Sep 22, 2021
@kovyrin kovyrin requested a review from a team September 23, 2021 15:10
ruflin
ruflin approved these changes Sep 29, 2021
View reviewed changes
Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Thanks for the fix.

@kovyrin kovyrin merged commit a07837e into elastic:master Sep 29, 2021
@kovyrin kovyrin deleted the kovyrin/sort-metrics branch September 29, 2021 14:41
mergify bot pushed a commit that referenced this pull request Sep 29, 2021
@kovyrin
Repeatable error messages for metric documentation checks (#28034)
92e9c46 
* Sort all keys before checking them for documentation to make sure missing documentation error messages are produced in a predictable and repeatable order.

This is really helpful when someone is trying to fix multiple issues and wants to make sure a specific issue is fixed before moving one to the next one. Otherwise, the errors are thrown in a random order and every time a new message is presented to the developer making it very confusing if the previous one had been resolved or not.

(cherry picked from commit a07837e)
@mergify mergify bot mentioned this pull request Sep 29, 2021
Closed
mergify bot pushed a commit that referenced this pull request Sep 29, 2021
@kovyrin
Repeatable error messages for metric documentation checks (#28034)
3fa000f 
* Sort all keys before checking them for documentation to make sure missing documentation error messages are produced in a predictable and repeatable order.

This is really helpful when someone is trying to fix multiple issues and wants to make sure a specific issue is fixed before moving one to the next one. Otherwise, the errors are thrown in a random order and every time a new message is presented to the developer making it very confusing if the previous one had been resolved or not.

(cherry picked from commit a07837e)
@mergify mergify bot mentioned this pull request Sep 29, 2021
Merged
kovyrin added a commit that referenced this pull request Oct 1, 2021
@mergify @kovyrin
Repeatable error messages for metric documentation checks (#28034) (#…
f6b7c7d 
…28190)

* Sort all keys before checking them for documentation to make sure missing documentation error messages are produced in a predictable and repeatable order.

This is really helpful when someone is trying to fix multiple issues and wants to make sure a specific issue is fixed before moving one to the next one. Otherwise, the errors are thrown in a random order and every time a new message is presented to the developer making it very confusing if the previous one had been resolved or not.

(cherry picked from commit a07837e)

Co-authored-by: Oleksiy Kovyrin <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Icedroid pushed a commit to Icedroid/beats that referenced this pull request Nov 1, 2021
@kovyrin
Repeatable error messages for metric documentation checks (elastic#28034
2aeff3e 
)

* Sort all keys before checking them for documentation to make sure missing documentation error messages are produced in a predictable and repeatable order.

This is really helpful when someone is trying to fix multiple issues and wants to make sure a specific issue is fixed before moving one to the next one. Otherwise, the errors are thrown in a random order and every time a new message is presented to the developer making it very confusing if the previous one had been resolved or not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruflin ruflin ruflin approved these changes

@jsoriano jsoriano Awaiting requested review from jsoriano

Assignees

@kovyrin kovyrin

Labels
backport-v7.16.0 Automated backport with mergify backport-v8.0.0 Automated backport with mergify enhancement Team:Elastic-Agent Label for the Agent team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kovyrin @elasticmachine @ruflin