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

Adding CloudEvents output option. #169

Merged
merged 1 commit into from
Jan 29, 2021
Merged

Adding CloudEvents output option. #169

merged 1 commit into from
Jan 29, 2021

Conversation

n3wscott
Copy link
Contributor

@n3wscott n3wscott commented Jan 11, 2021 •
edited

/kind feature
/area outputs

What this PR does / why we need it:

Adding CloudEvents Output support, starting with PoC of only via http binary mode.

@poiana poiana requested review from cpanato and Issif January 11, 2021 19:15
@poiana
Copy link

poiana commented Jan 11, 2021

Welcome @n3wscott! It looks like this is your first PR to falcosecurity/falcosidekick 🎉

@poiana poiana added the size/L label Jan 11, 2021
@n3wscott n3wscott force-pushed the add-cloudevents branch 2 times, most recently from 6b8b456 to 5b16f3b Compare January 11, 2021 21:23
@n3wscott n3wscott changed the title [WIP] Adding CloudEvents output option. Adding CloudEvents output option. Jan 11, 2021
Issif
Issif requested changes Jan 11, 2021
View reviewed changes
outputs/client.go Outdated Show resolved Hide resolved
outputs/cloudevents.go Outdated Show resolved Hide resolved
handlers.go Outdated Show resolved Hide resolved
outputs/cloudevents.go Outdated Show resolved Hide resolved
outputs/cloudevents.go Outdated Show resolved Hide resolved
outputs/client.go Show resolved Hide resolved
outputs/cloudevents.go Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
outputs/cloudevents.go Outdated Show resolved Hide resolved
outputs/cloudevents.go Outdated Show resolved Hide resolved
Issif
Issif requested changes Jan 11, 2021
View reviewed changes
README.md Outdated Show resolved Hide resolved
outputs/cloudevents.go Show resolved Hide resolved
@Issif Issif added this to In progress in 2.x via automation Jan 12, 2021
@Issif Issif added this to the 2.21.0 milestone Jan 12, 2021
KeisukeYamashita
KeisukeYamashita reviewed Jan 12, 2021
View reviewed changes
Copy link
Contributor

@KeisukeYamashita KeisukeYamashita left a comment

Choose a reason for hiding this comment

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

Overall LGTM but I have a comment, PTAL.

handlers.go Outdated Show resolved Hide resolved
@Issif
Copy link
Member

Issif commented Jan 12, 2021

/hold

The PR seems ok for me now, but I don't want an auto-merge before 2.20.0 is released. Maybe some fields have to be discussed too.

@poiana poiana added size/XXL and removed size/L labels Jan 12, 2021
@n3wscott
Copy link
Contributor Author

Rebased to HEAD.

I ran the lint tool on the PR and it updated the formatting on the README.md. It uses prettier.io 's rules so let me know if you do not like that.

@Issif
Copy link
Member

Issif commented Jan 12, 2021

@n3wscott I checked the README, the linting seems good for me, I'm not opiniated.

@n3wscott n3wscott force-pushed the add-cloudevents branch 2 times, most recently from 8ecdfe4 to 832fae8 Compare January 13, 2021 22:32
@Issif
Copy link
Member

Issif commented Jan 14, 2021

@n3wscott can you squash the commits please?

@n3wscott
Copy link
Contributor Author

Can you squash merge instead?

@Issif
Copy link
Member

Issif commented Jan 15, 2021

Can you squash merge instead?

Merges are automatically made after an approval by poiana bot, it doesn't squash merge as far I know.

adding cloudevents
6b2b704 
Signed-off-by: Scott Nichols <snichols@vmware.com>
Issif
Issif approved these changes Jan 28, 2021
View reviewed changes
Copy link
Member

@Issif Issif left a comment

Choose a reason for hiding this comment

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

thank you @n3wscott 🙏

@poiana poiana added the lgtm label Jan 28, 2021
@poiana
Copy link

poiana commented Jan 28, 2021

LGTM label has been added.

Git tree hash: 52f3e7099153a854e66b90c16b28b0ae068f2d8c

@poiana
Copy link

poiana commented Jan 28, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Issif

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Issif
Copy link
Member

Issif commented Jan 29, 2021

/unhold

@poiana poiana merged commit 0030250 into falcosecurity:master Jan 29, 2021
2.x automation moved this from In progress to Done Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KeisukeYamashita KeisukeYamashita KeisukeYamashita left review comments

@Issif Issif Issif approved these changes

@cpanato cpanato Awaiting requested review from cpanato

Assignees

@Issif Issif

Labels
approved area/outputs dco-signoff: yes kind/feature New feature or request lgtm size/XXL
Projects
Falcosidekick 2.x
Status: Done
2.x
Done
Milestone
2.21.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@n3wscott @poiana @Issif @KeisukeYamashita