Skip to content

Update package descriptions #1364

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

Merged
merged 12 commits into from
Jul 29, 2021

Conversation

bmorelli25
Copy link
Member

@bmorelli25 bmorelli25 commented Jul 28, 2021

Summary

This PR updates the package manifest.yml descriptions for every top-level package in the integrations repo. The goal of this PR is to add more descriptive descriptions as these will appear as a sub-heading on each integration's documentation page. In this first attempt, I've taken the following approach:

This Elastic integration collects + {logs/metrics} + from {integration name}

Open to any and all feedback on this as I'm not sure how these changes impact packages elsewhere (like in Kibana). Also, collecting is only one part of an integration. Do we want to add other verbs like parses or visualizes?

Other integrations

As far as I can tell, this PR updates all relevant integrations except APM, Endpoint, and Symantec -- which I presume live in other repos. I know where APM lives, and I think I can figure out where Endpoint and Symantec live.

Context

This is related to conversations in https://github.com/elastic/wordlake/pull/1 (sorry, internal link)

@elasticmachine
Copy link

elasticmachine commented Jul 28, 2021

💚 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-07-29T17:51:57.889+0000

  • Duration: 72 min 16 sec

  • Commit: 38977e5

Test stats 🧪

Test Results
Failed 0
Passed 2620
Skipped 3
Total 2623

Trends 🧪

Image of Build Times

Image of Tests

@@ -2,7 +2,7 @@ name: cef
title: CEF
version: 0.3.0
release: experimental
description: CEF Integration
description: This integration collects logs from common event format (CEF) instances
Copy link
Member

Choose a reason for hiding this comment

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

CEF isn't a product that you would run an "instance" of. It's a log format like JSON. So perhaps there is a better phrasing of this description.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, Andrew! I pulled "CEF instance" from L17 of the manifest file. Should I update that line as well to remove it? https://github.com/elastic/integrations/pull/1364/files#diff-966e13402568cef1412dea521600e7fb5172601510e51030c0bbb41d55351e43L17

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if that link will work. Here's a better one:

description: Collect logs from CEF instances

@bmorelli25 bmorelli25 requested review from goodroot and mtojek July 28, 2021 19:27
@bmorelli25 bmorelli25 marked this pull request as ready for review July 28, 2021 21:24
Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

In general we use elastic-package format to format sources of integrations.

@mtojek mtojek self-requested a review July 29, 2021 08:33
Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

@bmorelli25

We have requirement that CI must report green state to get the PR merged. In this case is red which means there is something wrong with this PR. There are problems with elastic_agent (missing manifest entry) and okta (missing manifest entry).

@bmorelli25
Copy link
Member Author

There are problems with elastic_agent (missing manifest entry) and okta (missing manifest entry).

Thanks! I think everything should pass this time 🤞

@bmorelli25
Copy link
Member Author

Noooooo. Green, but a change was merged to the system integration. I'll fix the conflicts and let the tests run again.

# Conflicts:
#	packages/system/changelog.yml
#	packages/system/manifest.yml
@mtojek mtojek self-requested a review July 29, 2021 15:45
Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

Remember to bump up package versions accordingly in case of conflicts.

LGTM, feel free to merge this PR if CI is happy.

@bmorelli25
Copy link
Member Author

Remember to bump up package versions accordingly in case of conflicts.

Yup. The merged update in this case happened to be a patch, so my minor update was still the latest -- just needed to merge in the new CL entry.

LGTM, feel free to merge this PR if CI is happy.

Thanks! I'll be clicking the button the second it's green :)

@bmorelli25
Copy link
Member Author

Test just timed out. Restarting.

@bmorelli25 bmorelli25 merged commit 5ec5208 into elastic:master Jul 29, 2021
@bmorelli25 bmorelli25 deleted the update-integration-descriptions branch July 29, 2021 19:05
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.

5 participants