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 FindPrometheusCpp file for prometheus-cpp library #106

Merged
merged 4 commits into from
May 31, 2024

Conversation

timwoj
Copy link
Member

@timwoj timwoj commented Apr 25, 2024

Related to zeek/zeek#3701

@Neverlord
Copy link
Member

I see a couple issues with this script:

The prometheus-cpp library installs CMake package files, so it is very confusing why there should be a find script for it at all. This would actually interfere with CMake's find_package if it weren't for the fact that this script "misspells" the library name. What's worse, it's name is Find..., but it doesn't try to find anything. The name obfuscates that this script actually adds sub-directories to the build with hard-coded paths. Due to the hard-coded paths, I don't think this is a good candidate for a re-usable script. This should probably just be a function in the main CMakeLists.txt in Zeek.

I've also outlined a re-usable way to integrate 3rd-party libraries in zeek/broker#402 (comment), which I'm happy to discuss as an alternative approach. 🙂

@timwoj timwoj force-pushed the topic/timw/prometheus-cpp branch from 6a5d060 to 34cf738 Compare May 23, 2024 01:27
@timwoj timwoj merged commit 5d9f05a into master May 31, 2024
1 check passed
@timwoj timwoj deleted the topic/timw/prometheus-cpp branch May 31, 2024 20:32
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.

2 participants