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

Moved consts files to a new module #974

Conversation

AlonZivony
Copy link
Contributor

No description provided.

@AlonZivony AlonZivony changed the title Moved consts files to new module Moved consts files to a new module Aug 30, 2021
@AlonZivony AlonZivony force-pushed the feature/tracee-ebpf/extract-consts-to-package branch from 1cde4ee to 0ecf642 Compare August 30, 2021 11:48
@AlonZivony AlonZivony marked this pull request as draft August 30, 2021 12:36
@CLAassistant
Copy link

CLAassistant commented Sep 14, 2021

CLA assistant check
All committers have signed the CLA.

@yanivagman
Copy link
Collaborator

@AlonZivony as we discussed offline, we can keep the consts where they are today, and add a new event id enum to external that will map the different event ids to some global event ids. I opened #1098 to track that.

Do you mind if we close this PR?

@AlonZivony
Copy link
Contributor Author

In my opinion this is not enough and this PR is needed.
The reason is that the internal consts of the events are needed for many things inside tracee-ebpf, and by exporting it to a new package other packages could be created too.
I just think that these consts does not belong to the main tracee package.

@yanivagman
Copy link
Collaborator

I'm not saying that we shouldn't split tracee-ebpf into packages, and we even have an open issue for that (#946, opened by you). But spliting into packages is something we should discuss first - to which packages do we want to split and why? I also don't think that a "consts" package is something that we want to have - it is too wide.
So my suggestion is to close this PR (anyways it is out of date and needs to be rebased as many things changed since), and to open a discussion (in the discussions tab) so we can decide about the packages we want to split tracee-ebpf to.

@AlonZivony
Copy link
Contributor Author

I agree to what you say.
I close the PR.

@AlonZivony AlonZivony closed this Oct 19, 2021
@AlonZivony AlonZivony deleted the feature/tracee-ebpf/extract-consts-to-package branch January 17, 2022 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants