Page MenuHomePhabricator

efl_canvas_animation_rotate: adjust API
ClosedPublic

Authored by bu5hm4n on Oct 10 2019, 6:47 AM.

Details

Summary

in task T8288 we concluded that a few APIs need to be adjusted in order
to stabelize animation classes at some point. This also adds a new macro
to eina in order to create EINA_VECTOR2 values more easily.

ref T8288

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.
bu5hm4n created this revision.Oct 10 2019, 6:47 AM
bu5hm4n requested review of this revision.Oct 10 2019, 6:47 AM

I like this addition. I just prefer when API being added to Eina are done in two step, one patch for eina and another one rolling it in everywhere. For the current case, it is just a one line, so I won't oppose this patch landing, but I just wanted to let you know my preference for future reference.

src/lib/eina/eina_vector.h
35

Is our preferred doc cop @segfaultxavi happy here?

segfaultxavi requested changes to this revision.Oct 10 2019, 1:06 PM
segfaultxavi added inline comments.
src/lib/eina/eina_vector.h
35

NO, obviously. But I was unsure on how we document macros so I let it slip :)

This revision now requires changes to proceed.Oct 10 2019, 1:06 PM
bu5hm4n requested review of this revision.Oct 28 2019, 12:44 AM
bu5hm4n added inline comments.
src/lib/eina/eina_vector.h
35

None of these macros is documented in eina. I do not want to start a discussion here on how to document them. Futher more, its a Vector 2 with 2 fields, what would you think this is doing ? brewing some coffee ? :P

segfaultxavi resigned from this revision.Oct 28 2019, 3:27 AM
segfaultxavi added inline comments.
src/lib/eina/eina_vector.h
35

Hrmpffff mumble mumble mumble

I am not happy but yeah, OK. But at some point ALL public-facing API (macros included) will have to be documented.

zmike accepted this revision.Oct 30 2019, 5:34 AM

Seems fine, you can clean the nits up when you land it.

src/lib/evas/canvas/efl_canvas_animation_rotate.eo
22

What does that even mean tho?

30

center_point ?

This revision is now accepted and ready to land.Oct 30 2019, 5:34 AM
Jaehyun_Cho accepted this revision.Nov 1 2019, 4:14 AM

Seems ok to me.
Same comment with @zmike about "center_pointer".

Closed by commit rEFLf47b9277453e: efl_canvas_animation_rotate: adjust API (authored by Marcel Hollerbach <mail@marcel-hollerbach.de>, committed by segfaultxavi). · Explain WhyNov 4 2019, 4:19 AM
This revision was automatically updated to reflect the committed changes.