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

Introduction of an optional short field description #330

Merged
merged 14 commits into from
Feb 22, 2019

Conversation

webmat
Copy link
Contributor

@webmat webmat commented Feb 18, 2019

This PR adds a short: description to the YAML format for defining fields. All fields who need a short description for easy display in cramped spaces now have one.

This PR does not address the outputs which would benefit from this new short description. This should be addressed in a separate PR per output. PRs welcome, BTW ;-)

In reviewing all of these field descriptions, some main descriptions were adjusted as well.

A few other concerns have been addressed also:

  • Replace incorrect ec2 example in cloud.provider with aws.
  • Add pointer in description of http field set to url field set.
  • Split up all field set definitions in multiple paragraphs.

I can extract to their own PR, if needed.

Questions

  • @AndrewGoldstein Do you feel like we need a "short" description for "key" descriptions as well? (E.g. client field set description)
    • Yes we need it. Obvious when looking at fieldset headings in the asciidoc format. They're part of this PR

@webmat webmat self-assigned this Feb 18, 2019
@webmat webmat force-pushed the short-description branch 2 times, most recently from 6c47109 to 5f50b7e Compare February 18, 2019 21:06
@MikePaquette MikePaquette self-requested a review February 22, 2019 14:09
Copy link
Contributor

@MikePaquette MikePaquette left a comment

Choose a reason for hiding this comment

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

Approved with the minor fixes and changes I committed.

@karenzone
Copy link
Contributor

@dedemorton Here's @webmat 's work on toward giving us a shortdesc.

@@ -20,7 +20,7 @@
type: keyword
short: Name of the cloud provider.
description: >
Name of the cloud provider. Example values are aws, azure, gce, or
Name of the cloud provider. Example values are aws, azure, gcp, or
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch

Copy link
Contributor Author

@webmat webmat left a comment

Choose a reason for hiding this comment

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

Since it's my own PR, I can't mark as Approved. But I agree with your changes, Mike :-) Thanks.

I'll merge.

CHANGELOG.md Outdated Show resolved Hide resolved
Mathieu Martin and others added 13 commits February 22, 2019 14:31
Also:

- Replace incorrect `ec2` example in `cloud.provider` with `aws`.
- Add pointer in description of `http` field set to `url` field set.
This will improve consistency with how we've been formatting field definitions.
Comes with minor description adjustments as well.
typo - "field" to "fields" in message description
There's now two fixes in the field description
@webmat webmat merged commit 34b391c into elastic:master Feb 22, 2019
@webmat webmat deleted the short-description branch February 22, 2019 20:41
@@ -17,8 +18,9 @@
level: extended
example: ec2
Copy link
Contributor

Choose a reason for hiding this comment

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

@webmat Sorry I just saw this PR for some reason... Thanks for making the changes! I just found this ec2 still in provider example. This probably should be aws instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh jeez, we fixed the example in the text, but not in the example field, haha! Thanks for pointing it out 👍

webmat pushed a commit to webmat/ecs that referenced this pull request Feb 28, 2019
PR elastic#330 only fixed the text in the description :-)
webmat pushed a commit to webmat/ecs that referenced this pull request Mar 4, 2019
PR elastic#330 only fixed the text in the description :-)
webmat added a commit that referenced this pull request Mar 5, 2019
PR #330 only fixed the text in the description :-)
webmat added a commit to webmat/ecs that referenced this pull request Mar 5, 2019
…n to fields and fieldsets (elastic#330)

Additional things were fixed, while playing in these descriptions:

- Some definitions were adjusted in terms of wording and in terms of separating paragraphs (double \n)
- `cloud.provider` examples fixed to mention the actual provider, not a service like "ec2"
- Add pointer in `http` field description to `url`
- Lowercased the `log.level` examples. The field doesn't technically have the requirement, but at least we can start guiding in that direction for the future.
webmat added a commit that referenced this pull request Mar 5, 2019
…elds and fieldsets (#330) (#362)

Backport of PR #330 to 1.0 branch. Original message:

Additional things were fixed, while playing in these descriptions:

- Some definitions were adjusted in terms of wording and in terms of separating paragraphs (double \n)
- `cloud.provider` examples fixed to mention the actual provider, not a service like "ec2"
- Add pointer in `http` field description to `url`
- Lowercased the `log.level` examples. The field doesn't technically have the requirement, but at least we can start guiding in that direction for the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cloud.provider example under Cloud fields
4 participants