Page MenuHomePhabricator

eo: make efl_future_then have a data pointer in addition of the object pointer.
ClosedPublic

Authored by cedric on Wed, Nov 28, 9:38 AM.

Details

Summary

In the case when you have multiple future in flight related to one object, you
couldn't use the previous version of efl_future_then. Now all function calls
take a void* pointer that allow multiple future to have their private data
request data accessible in all the callback.

This should not break released API as Eo.h is not released yet and so
was efl_future_Eina_FutureXXX_then.

Depends on D7332

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
cedric created this revision.Wed, Nov 28, 9:38 AM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

cedric requested review of this revision.Wed, Nov 28, 9:38 AM
segfaultxavi requested changes to this revision.Thu, Nov 29, 1:57 AM

This is modifying an existing (unreleased) API. Could you add an usage example, or, even better, a unit test? It would also help me better understand the changes since in this diff I do not see where is the data provided.

Also, it would have been nice to put the whitespace changes in a different patch.

This revision now requires changes to proceed.Thu, Nov 29, 1:57 AM

I did update the doxygen with the example, but it is true that there isn't any tests of efl_future_then. Will fix that.

cedric updated this revision to Diff 17693.Thu, Nov 29, 3:09 PM
cedric edited the summary of this revision. (Show Details)
cedric added a reviewer: bu5hm4n.

Fix tests cases too.

Sorry, I do not see any Doxygen comment with an example.

cedric updated this revision to Diff 17783.Wed, Dec 5, 11:34 AM

Rebase.

cedric updated this revision to Diff 17791.Wed, Dec 5, 12:13 PM

Nothing.

segfaultxavi requested changes to this revision.Thu, Dec 6, 1:27 AM

@cedric What about those examples and tests?

This revision now requires changes to proceed.Thu, Dec 6, 1:27 AM

Oh, they are in D7393, now I see them. Still, I cannot test this patch because the one it depends on (D7332) is failing to compile.

cedric updated this revision to Diff 17819.Thu, Dec 6, 9:39 AM

Nothing.

segfaultxavi accepted this revision.Fri, Dec 7, 2:59 AM
This revision is now accepted and ready to land.Fri, Dec 7, 2:59 AM
This revision was automatically updated to reflect the committed changes.