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

Improve descriptions of config fields to select objects #14045

Merged

Conversation

iliakur
Copy link
Contributor

@iliakur iliakur commented Feb 28, 2023

What does this PR do?

Motivation

Support case. I noticed the descriptions could use some love:

  • reduce duplication by stating something once and then referencing that later
  • remove some confusing wording
  • for some fields the description now reflects the actual implementation

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached
  • If the PR doesn't need to be tested during QA, please add a qa/skip-qa label.

@iliakur iliakur requested review from a team as code owners February 28, 2023 09:22
Copy link
Contributor Author

iliakur commented Feb 28, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@codecov
Copy link

codecov bot commented Feb 28, 2023

Codecov Report

Merging #14045 (308976c) into master (2e0149f) will not change coverage.
The diff coverage is n/a.

❗ Current head 308976c differs from pull request most recent head fac8725. Consider uploading reports for the commit fac8725 to get more accurate results

Flag Coverage Δ
rabbitmq 95.63% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

FlorentClarret
FlorentClarret previously approved these changes Feb 28, 2023
Copy link
Member

@FlorentClarret FlorentClarret left a comment

Choose a reason for hiding this comment

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

Looks good to me

kayayarai
kayayarai previously approved these changes Feb 28, 2023
Copy link
Contributor

@kayayarai kayayarai left a comment

Choose a reason for hiding this comment

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

Grammar/style fixes, but 👍

rabbitmq/assets/configuration/spec.yaml Outdated Show resolved Hide resolved
rabbitmq/assets/configuration/spec.yaml Outdated Show resolved Hide resolved
rabbitmq/assets/configuration/spec.yaml Outdated Show resolved Hide resolved
rabbitmq/datadog_checks/rabbitmq/data/conf.yaml.example Outdated Show resolved Hide resolved
rabbitmq/datadog_checks/rabbitmq/data/conf.yaml.example Outdated Show resolved Hide resolved
@iliakur iliakur dismissed stale reviews from kayayarai and FlorentClarret via fac8725 February 28, 2023 16:57
@github-actions
Copy link

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@iliakur
Copy link
Contributor Author

iliakur commented Feb 28, 2023

Thanks @kayayarai I'll go ahead with the merge without waiting for a reapproval since I applied your suggestions :D

@iliakur iliakur merged commit 6240156 into master Feb 28, 2023
@iliakur iliakur deleted the ik/Improve_descriptions_of_config_fields_to_select_objects branch February 28, 2023 16:58
@iliakur iliakur mentioned this pull request Feb 28, 2023
5 tasks
@hithwen hithwen added the qa/skip-qa Automatically skip this PR for the next QA label Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation integration/rabbitmq qa/skip-qa Automatically skip this PR for the next QA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants