On Sun, Feb 3, 2013 at 10:58 AM, fabien pichot <pichot.fab...@gmail.com> wrote: > Hello, > > I didn't lurk that ML that much, so I'm not really sure about the way to send > patches. But anyway, attached is a patch that makes the error codes public and > add two functions to get the error and the http request from the evrpc_status > structure.
Hi, Fabien! Attachments are fine. Here's a quick review: 1) Turning the values from #define into an enum will create problems for users of C++ compilers that like to give warnings about int/enum conversions. Probably not a great idea, since it would break existing code. 2) The names for these functions should probably be evrpc_status_get_FIELD, rather than evrpc_status_FIELD. (yes, some older accessor function names in libevent don't follow the TYPE_get_FIELD convention -- but we're trying not to do that with new function names.) 3) The new functions and values need documentation in the header. 4) There's no unit test for the evhttp_request accessor. 5) The formatting doesn't match our conventions: we try to put a newline between return types and function names only when defining a function, not when declaring it. If you're able to fix these in a new patch, that would be excellent. best wishes, -- Nick *********************************************************************** To unsubscribe, send an e-mail to majord...@freehaven.net with unsubscribe libevent-users in the body.
