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

Add uthash #203

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Add uthash #203

merged 2 commits into from
Jan 24, 2024

Conversation

tytan652
Copy link
Contributor

Description

Add uthash to obs-deps.

Yes, they should be installed in the include directory without any subdirectory to align with Linux distros.

Motivation and Context

Reduce in-tree vendored deps in OBS Studio repo.

How Has This Been Tested?

CI builds artifacts on my fork PR, they contain headers and license files.

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@derrod
Copy link
Member

derrod commented Jul 24, 2023

Is there a specific reason to include the other headers? We only need uthash.h and there should be no interdependence between them.

@tytan652
Copy link
Contributor Author

I just did it like a packager should do it since all the header are part of uthash.

@tytan652 tytan652 force-pushed the uthash branch 2 times, most recently from e33a5f7 to cc3edad Compare August 3, 2023 16:07
Copy link
Member

@PatTheMav PatTheMav left a comment

Choose a reason for hiding this comment

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

Should be good. I also agree with copying all related files to make it a "default" distribution.

The compiler will pull in files as needed.

Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

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

Seems fine at a glance. Will merge when we get to a merge window.

@RytoEX RytoEX merged commit 9e0ea1f into obsproject:master Jan 24, 2024
21 checks passed
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