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

Remove leftover FreeBSD related code and documentation #7739

Merged
merged 6 commits into from
Sep 27, 2022

Conversation

zhumo
Copy link
Contributor

@zhumo zhumo commented Aug 18, 2022

Removing freebsd from:

  1. documentation
  2. CMakelists
  3. freebsd .tables and .cpps
  4. generators

Copy link
Member

@directionless directionless left a comment

Choose a reason for hiding this comment

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

Hrm. This is a good question. In the past, our theory had been that someone might revisit this, so we should leave the code in place. But, it's not wrong to re-evaluate it, and simpler code has some advantages. I am ambivalent.

@Smjert
Copy link
Member

Smjert commented Aug 18, 2022

Hrm. This is a good question. In the past, our theory had been that someone might revisit this, so we should leave the code in place. But, it's not wrong to re-evaluate it, and simpler code has some advantages. I am ambivalent.

I vote to remove it for now; the code will always exists in the git history for reference.

There hasn't been a direct interest in it (and with direct I mean a contributor with the interest, ability and time to support it), and we are not updating it/keeping it working since the start of 4.x, so it's just dead code.

Additionally, we don't have the infra for the CI to run tests, and it would be an additional maintenance burden for whoever finds itself having to update a library, and reconfigure it on all the platforms and architectures.

@zhumo
Copy link
Contributor Author

zhumo commented Aug 18, 2022

In other projects, we've generally followed the idea that it will exist in git history so we can revive it there, where necessary, as Stefano is saying. Deleting code is a joy!

That said, at the very least, we would still want to get rid of the .table files right? Happy to pare this down in some way, wherever you guys decide. But I figure I would start here by submitting the maximalist approach.

@mike-myers-tob mike-myers-tob added ready for review Pull requests that are ready to be reviewed by a maintainer deprecation Relating to function deprecation labels Aug 18, 2022
@directionless
Copy link
Member

Between @Smjert and @zhumo that's two votes to remove, and none to keep. I'm onboard!

@zhumo -- you mentioned not accidentally removing windows, other than that it's seems great.

directionless
directionless previously approved these changes Aug 24, 2022
Copy link
Member

@directionless directionless left a comment

Choose a reason for hiding this comment

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

I think that's okay?

specs/CMakeLists.txt Outdated Show resolved Hide resolved
@zhumo
Copy link
Contributor Author

zhumo commented Aug 25, 2022 via email

@mike-myers-tob mike-myers-tob changed the title Remove freebsd Remove leftover FreeBSD related code and documentation Aug 29, 2022
@directionless directionless merged commit b57cae8 into osquery:master Sep 27, 2022
@zhumo zhumo deleted the remove-freebsd branch October 11, 2022 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation Relating to function deprecation FreeBSD ready for review Pull requests that are ready to be reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants