Hey,
Thanks a lot for reviewing and sorry for the late reply, I was AFK for a
couple of days.
On 08/04/12 10:59, Vincent Torri wrote:
> eobj.h --> Eobj.h (like all other EFL)
> missing EAPI definition in eobj.h
Thanks. It evolved out of a POC so I a lot is bad in that regard. :)
I renamed eobj.h, but I don't see any missing EAPI. Maybe you reviewed a
very old version of the code?
>
> eina_log.h inclusion useless in eobj.c
Removed that, thanks.
> missing EAPI in front of eobj_ref() in eobj.c
Fixed.
> I like to give a namespace to variables even if they are static (like
> classes --> _eobj_classes, same for classes_last_id. You did it for
> the log domain, so do it for the others)
I don't care one way or the other, but I changed it.
> missing _ in front of eobj_class_constructor() and _eobj_class_destructor()
Yes, I know, the reason is that I recently changed them to static and I
haven't decided yet. It's not a strict convention anyway, so I didn't
bother until I really decide.
> wrong indentation
> struct { should be :
> struct
> {
Oops, yeah, did it wrong there.
>
> better organise eobj.c (a bit messy). In order:
>
> macro
> typedef
> struct
> static functions
> API
Yes, should be played with a bit, I know.
>
> same for eobj.h (without static fcts of course)
I know.
As I said in my first email, internally it's a bit messy.
>
> question : why
>
> struct _Eobj
> {
> Eobj *parent;
> ***
> };
>
> instead of
>
> struct _Eobj
> {
> Eobj parent;
> ***
> };
It's a pointer to the parent object in the hierarchy, not "struct
inheritance". As you can see, it's of the same type of the struct
itself, making it Eobj parent doesn't make sense and won't even compile.
> it should compile on Windows at first sight
>
That's what I think as well. Lets hope/test. :)
Thanks a lot for your review.
--
Tom.
------------------------------------------------------------------------------
Better than sec? Nothing is better than sec when it comes to
monitoring Big Data applications. Try Boundary one-second
resolution app monitoring today. Free.
http://p.sf.net/sfu/Boundary-dev2dev
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
