-
-
Notifications
You must be signed in to change notification settings - Fork 290
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 meson buildsystem #275
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this contribution!
I'm generally not opposed to the change. This of course does slightly increase the maintenance burden, I hope you stick around to help with that as well :).
I added some comments
meson.build
Outdated
# See COPYING for more information about licensing | ||
# | ||
|
||
project('nethogs',['c','cpp'],version : '0.8.7', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
specifying '0.8.7' here seems a bit weird, since the current main branch is '0.8.7 and a bit'. Is there any convention in the meson ecosystem to deal with the version differently, perhaps similar to what the Makefile does?
Also, the formatting seems somewhat haphazard (e.g., when do use newlines and when not). Is there any formatting tool for meson build files? Did you use it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately there is not a tool, but here is the style guide: https://mesonbuild.com/Style-guide.html
Let me see how I can fix the version. Probably I can get it from your version script
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added the detection of the version from the tag as in the Makefiles.
meson_options.txt
Outdated
@@ -0,0 +1 @@ | |||
option('enable-app', type: 'boolean', value: false, description: 'Enable CUI app') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It kinda feels like the defaults should be the other way around: nethogs is primarily a CUI tool, and having it as a library is a somewhat experimental edge case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, I have added both options (I will be using the library)
594f9fe
to
8bd8eb7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, thanks!
This feature:
Instructions: