On Friday, December 28, 2018 2:32 AM, Carsten Haitzler <ras...@rasterman.com> wrote: > On Thu, 27 Dec 2018 13:26:19 -0500 Mike Blumenkrantz > michael.blumenkra...@gmail.com said: > > > Hi, > > Thanks for taking the time to look into this issue. There's been an amount > > of discussion and back-and-forth patching related to it for some time now, > > and it's great to see someone taking some initiative here. > > Looking over this patch, I'm unsure whether the method used for resolving > > the issue is indeed the optimal one. Given the already-existing discussion > > indeed my log comments question the original commit idea - but i thought this > was a bit better than reverting since i had found a non-revert fix. my take is > that "wrapping" eina promise like this is kind of problematic as it means we > now have some "forbidden" calls that are needed to do the wrapping properly > (the ones i added).
Yes, I think the API of Eina promise doesn't work well for the ownership of the data. It is also not matching the rest of our API in that regard. I have created task T7530 where I put an idea of what need to be fixed and to discuss this further. > > going on related to this topic, I think this is a patch which would have > > benefited from review prior to being landed. In particular, I'd have > > suggested that unit tests be added for this case--we have somewhat > > it's still up in the air as to if this is the right way in the end - but i'm > not sure how that helps as these are "public api's" only in as much as we need > them internally but due to having multiple libraires ... we need to expose > them. the documentation reflects that. For future reference, we do have the concept of internal API across our libraries. This internal API are to be defined inside eina_internal.h for Eina for example. Not perfect as the symbol is obviously public, but people will know right away that they are using a symbol that is not documented. Cedric _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
