Skip to content

Conversation

@adriansr
Copy link
Contributor

@adriansr adriansr commented Dec 27, 2021

This helps the type to be reused by other packages in the future.

First part of refactoring #585 into smaller PRs.

@adriansr adriansr added enhancement New feature or request Team:Elastic-Agent Label for the Agent team Team:Security-External Integrations labels Dec 27, 2021
@adriansr adriansr requested a review from mtojek December 27, 2021 08:44
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

@elasticmachine
Copy link
Collaborator

elasticmachine commented Dec 27, 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: 2022-01-11T10:35:25.131+0000

  • Duration: 26 min 51 sec

  • Commit: f3a8c7e

Test stats 🧪

Test Results
Failed 0
Passed 459
Skipped 0
Total 459

🤖 GitHub comments

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

  • /test : Re-trigger the build.

This helps the type to be reused by other packages
}

// FileName returns the original filename associated with the pipeline.
func (p *Resource) FileName() string {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Filename

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

)

// Resource represents a pipeline resource loaded from a file
type Resource struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking loud now, as the Resource may be too vague.

An alternative approach would be:

  • create the ingest package in the internal/elasticsearch directory
  • add pipelines.go
  • create the structure Pipeline.

or just move this file (with unit test) to the internal/elasticsearch directory, and rename to Pipeline. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@adriansr adriansr requested review from efd6 and mtojek January 11, 2022 11:04
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.

Nice and clear, well done.

@mtojek mtojek merged commit 35c3beb into elastic:master Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Team:Elastic-Agent Label for the Agent team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants