Denys, thank you very much for the Catcut plugin. I finally got a chance to look at it, and do some simple testing. It looks pretty good.
I've filled out the add_attachment code in a patch below. I would appreciated it if you would review it, and check it in if it's acceptable.
In writing this code I have come to believe that all of the plugins would benefit from having common bits of code factored out into a plugin support library, rather than cut and pasted from one plugin to another. I'm curious what other ABRT developers think of this.
Also in looking at several plugins, I've noticed that different plugins have different policies for reporting errors, warnings, informational, and debugging messages. Sometimes exceptions are used, sometimes calls to 'update_client' and sometimes calls to 'log'. Is there a pattern/policy that I just haven't figured out yet, or is this still in flux?
Thanks again.
-gavin...
On Thu, 2009-11-05 at 12:14 -0500, Gavin Romig-Koch wrote:
Denys, thank you very much for the Catcut plugin. I finally got a chance to look at it, and do some simple testing. It looks pretty good.
I've filled out the add_attachment code in a patch below. I would appreciated it if you would review it, and check it in if it's acceptable.
Will do later today. If I don't, yell at me.
In writing this code I have come to believe that all of the plugins would benefit from having common bits of code factored out into a plugin support library, rather than cut and pasted from one plugin to another. I'm curious what other ABRT developers think of this.
We already have it. See abrtlib.h, lin/Utils/*.{h.cpp}
Also in looking at several plugins, I've noticed that different plugins have different policies for reporting errors, warnings, informational, and debugging messages. Sometimes exceptions are used, sometimes calls to 'update_client' and sometimes calls to 'log'. Is there a pattern/policy that I just haven't figured out yet, or is this still in flux?
It's in flux, and it used to be much worse than how it looks now.
exceptions were there before my time. I don't like them. Maybe remove?
currently, we have:
log(fmt...) just writes to the log (stderr / syslog / dev/null, depending on the application's setup)
error_msg(fmt...) the same as log, but semantic is that this is an _error_, not just random bit of info
[p]error_msg[and_die] == [append errno string to] error_msg [and die].
update_client(str) send this string to client over DBUS (for logging or display by clients)
warn_client(str) send this string to client over DBUS. This is an error message.
Plans:
Minimum: make warn/update_client() take printf formats.
I am thinking about making XXXerror_msgXXX automatically do warn_client(). There is rarely (never?) a situation where we have an error we wish to log on server side, but _dont_ want to let client know about it.
log() will log only on server side - used for debugging, thus not a good idea to swamp clients with these msgs.
update_client() will perhaps remain, it does not map nicely to log() or error_msg().
-- vda
On Fri, 2009-11-06 at 16:16 +0100, Denys Vlasenko wrote:
Also in looking at several plugins, I've noticed that different plugins have different policies for reporting errors, warnings, informational, and debugging messages. Sometimes exceptions are used, sometimes calls to 'update_client' and sometimes calls to 'log'. Is there a pattern/policy that I just haven't figured out yet, or is this still in flux?
It's in flux, and it used to be much worse than how it looks now.
exceptions were there before my time. I don't like them. Maybe remove?
currently, we have:
log(fmt...) just writes to the log (stderr / syslog / dev/null, depending on the application's setup)
error_msg(fmt...) the same as log, but semantic is that this is an _error_, not just random bit of info
[p]error_msg[and_die] == [append errno string to] error_msg [and die].
update_client(str) send this string to client over DBUS (for logging or display by clients)
warn_client(str) send this string to client over DBUS. This is an error message.
Plans:
Minimum: make warn/update_client() take printf formats.
I am thinking about making XXXerror_msgXXX automatically do warn_client(). There is rarely (never?) a situation where we have an error we wish to log on server side, but _dont_ want to let client know about it.
log() will log only on server side - used for debugging, thus not a good idea to swamp clients with these msgs.
update_client() will perhaps remain, it does not map nicely to log() or error_msg().
It is committed to git now.
-- vda
On Fri, 2009-11-06 at 16:16 +0100, Denys Vlasenko wrote:
On Thu, 2009-11-05 at 12:14 -0500, Gavin Romig-Koch wrote:
Denys, thank you very much for the Catcut plugin. I finally got a chance to look at it, and do some simple testing. It looks pretty good.
I've filled out the add_attachment code in a patch below. I would appreciated it if you would review it, and check it in if it's acceptable.
Will do later today. If I don't, yell at me.
Applied, please test current abrt git. -- vda
crash-catcher@lists.fedorahosted.org