search

maedoc / libtvb

TVB C library
6 stars 8 forks source link

Added function log_fun.c #118

Open paramhanji opened 8 years ago

paramhanji commented 8 years ago

I've added a function a replace existing typedef as mentioned in issue #88. Kindly review it and tell me if any changes need to be made.

AbheekG commented 8 years ago

The implementation can be done with more care, doesn't seem to be correct.

maedoc commented 8 years ago

@catchchaos thanks for starting this; as @abheekg comments, there are some details to fix.

In general, make sure tests run on your machine (make tests && ./tests) and see and fix Travis' errors.

AbheekG commented 8 years ago

typedef int (*sd_log_msg_fp)(const char *fmt, ...); can be used instead of printf_t

maedoc commented 8 years ago

@catchchaos the target for the log messages is currently defined in sddekit_api.h, but this type signature should change to just taking a string so that the log handler doesn't have to do formatting.

Do what you can, ask more questions if you like and I'll also fetch your branch and revise it.

paramhanji commented 8 years ago

@abheekg Oops I forgot ensure that stdarg was included. Thanks and let me know what else I could do.

@maedoc Sure next time I'll make sure I run the tests first. Sorry for this. I'll make the changes as soon as I can.

maedoc commented 8 years ago

@catchchaos @abheekg no rush I won't have time to review today

paramhanji commented 8 years ago

I wasn't sure how to invoke the logger. @abheek I meant to put in a comment for the same, but I forgot @maedoc Should the file be extended to include getters and setters for the verbose and quiet flags as well? I noticed that sd_log.c has this functionality.

maedoc commented 8 years ago

@catchchaos I had in mind just adding to sd_log.c not replacing it. This is not a well-documented part so try to follow the details I mentioned in #88 and otherwise ask questions, push new commits so we can see where you are.

paramhanji commented 8 years ago

@maedoc I implemented everything that you described in issue #88. sd_log.c compiles fine on its own. However, make says there's an error in the file. Can I make a pr? I'm not sure why the make error is occuring. The error code is 1 indicating that the file needs to be rebuilt.

maedoc commented 8 years ago

Can you commit your changes and push to this PR please?

paramhanji commented 8 years ago

I commited the changes yesterday. Have a look when you can :)