-
Notifications
You must be signed in to change notification settings - Fork 51
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
Split up ovis_log.h #1357
Comments
@morrone, why is this necessary/advisable. Sorry I'm not getting this. The clients that are registering/destroying are the same clients that are initializing opening/flushing and publishing messages. What is the use-case I'm missing? |
Most "clients" (what I called "log service users" in 2), do not do the things in 1. I'm talking about most of the sampler plugins, and possibly most plugins in general. Likely various parts of ldms itself only use the log service instaniated once somewhere else as well. Samplers don't so (1), ldmsd internally does (1), and plugins only do (2). Because most sampler plugins only use three functions:
Something else, ldmsd, created the log service that the plugin is registering itself with, and then using. Basically, there are things that instantiate a log files service (ldmsd), and that is (1). Then there are things that use log file services instances (2). But right now everything is squashed together and less clear for uses of the code than they could be. |
@morrone, I get it, but we're talking about creating a .h file that has three functions in it. It seems like we're adding to the confusion, not ameliorating it (is that a word?) |
@morrone, I do think better doc is appropriate/required. |
I'm not sure why the number of functions is an issue. We're not going to run out space for header files. :) This is all about code abstraction. Users of a logging service shouldn't need to be exposed to the all the interfaces needed to create and run a logging service. |
PR #1344 proposes a good change to ovis_log.h of converting "struct ovis_log_s" into an opaque type.
I think there is another change needed to split the header file up into at least two parts:
The text was updated successfully, but these errors were encountered: