Page MenuHomePhabricator

bu5hm4n (Marcel Hollerbach)Administrator
spacecowboy

Projects (8)

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Thursday

  • Clear sailing ahead.

User Details

User Since
Dec 2 2013, 11:58 AM (311 w, 1 d)
Roles
Administrator
Availability
Available

Recent Activity

Today

bu5hm4n added a comment to D10607: Efl Canvas Text : Modify Style Property.

There is no need to discuss something like "please make use of a macro here" or "this should be tested", nor are these *suggesstions*, they changes that i am requesting.
I am also not saying that discussions are not allowed, but coming to a discussion with a argument "its also like this somewhere else" is not going to be usefull at any rate.
Replying to a inline comment where we would like to have a change with "we will do it later" is also not really a discussion, why would you start later on again on old code file fixing something that was said ahead of time, even if it could have been fixed before this piece of code would have even landed ? That makes no sense to me. Fix it now, no need to discuss things, no need to revisit things, and more testing, review can happen.

Tue, Nov 19, 11:07 AM · efl
bu5hm4n added a comment to D10607: Efl Canvas Text : Modify Style Property.

Okay, i am starting to be annoyed by replies like this. I am called here to do review, i do review, and all that is coming back is "we will do it later" "its used somewhere else like this" "we should not document concepts like this".
There is no golden rule about what we declare as easy to read code, however, this block is the perfect example of what it is NOT, it takes ONE single macro to get this cleaned up, where you have something like MY_MACRO("off", EVAS_TEXT_STYLE_PLAIN, EFL_TEXT_STYLE_EFFECT_TYPE_NONE) one call for each line up there. Which is WAY more readable than what is there right now. We also do EXACTLY this macro stuff ALL over the place, so the argument of "used in other part of text" is completely irrelevant and does not mean anything.
Other people have to READ that code again, have to MAINTAIN that code in some way, and someone on a bug-hunt will likely come across it, why should we make the life of these people harder ?
Have you ever heard of the "Broken windows theory" ? Whenever there is something broken or ugly, someone else, who does not want to make it any better will exactly refer to this code "eh, it was done like this before", could you please try to get off that attitude a bit ? This code here will last a long time, and why would you want to start with a hard-to-read version if you can invest 5 min. and make it nice ? (Same applies to other comments and revisions regarding tests, or string parsing, or or or )

Tue, Nov 19, 9:27 AM · efl
bu5hm4n updated the diff for D10615: introduce efl_canvas_object_animation.

make xavi happy

Tue, Nov 19, 9:08 AM · efl
bu5hm4n accepted D10690: docs: Copy all images to output folder.

I am pretty much fine with this,i think we can still move to python if there are issues. I am also not sure if the doc build works anywhere except linux ... :/

Tue, Nov 19, 3:04 AM · efl
bu5hm4n added a comment to D10679: Revert "efl_ui/layout: selectively inhibit theme,changed event".

No problem, @zmike is on vacation ... :)

Tue, Nov 19, 2:29 AM · efl
bu5hm4n added a comment to D10607: Efl Canvas Text : Modify Style Property.

@bu5hm4n

can you check that all branches of the parsing function is checked and tested (also with asan enabled)?

I have tested only new code.

Tue, Nov 19, 2:17 AM · efl
bu5hm4n requested changes to D10646: Efl.Text.Attribute_Factory.

I think the errors in efl_text_attribute_factory.c should be done using EINA_SAFETY_ON_FALSE_RETURN. Which will also print the conditions, otherwise these errors are not really helpful.

Tue, Nov 19, 1:49 AM · efl

Yesterday

bu5hm4n updated the diff for D10615: introduce efl_canvas_object_animation.

more docs

Mon, Nov 18, 5:53 AM · efl
bu5hm4n added a comment to D10646: Efl.Text.Attribute_Factory.

As a first review, its quite hard to review here anything, as cursor changes seems to have mixed into this here.

Mon, Nov 18, 1:52 AM · efl
bu5hm4n added a comment to D10646: Efl.Text.Attribute_Factory.

Is it possible that this here is totally mixed up ? there are also changes from the cursor in here ?

Mon, Nov 18, 1:26 AM · efl

Sun, Nov 17

bu5hm4n accepted D10662: slider: fix value error from step.

Thank you :)

Sun, Nov 17, 11:09 PM · efl
bu5hm4n added a comment to T8470: Building efl on x86 error: symbol lookup error: /eolian_gen: undefined symbol: eolian_state_new.

No no, you should not, I just wondered if you did before ... (but you you didn't, which is correct).

Sun, Nov 17, 8:03 AM · efl (efl-1.24), Restricted Project
bu5hm4n added a comment to D10542: Efl.Text.Cursor.

Okay, i think it would be quite good to add these tests (about creating the textobject by hand) and then calling API without a text object, just to ensure, that there are no crashes etc.
It also might be worth thinking about if its acceptable that there are no errors, maybe this is hiding some bugs ?
Last but not least, if there are errors with ASAN (that are no leaks), then it does not really matter if this is caused by this patch or not, its not really acceptable to have a new commit that first leads to crashes in things that have been working before. These things needs clarification beforehand.

Sun, Nov 17, 6:05 AM · efl
bu5hm4n added a comment to T8470: Building efl on x86 error: symbol lookup error: /eolian_gen: undefined symbol: eolian_state_new.

I am not sure why people are seeing this (you are not the first one), there is not a single call within the efl-build that is loading old e* libraries from the system to compile based on that. And the -I / -L flags are added as first by meson, only the user CFLAGS go higher in the priority, have you set custom CFLAGS that are including the libraries from these directories? (Or is your compiler not honoring the ordering of flags like (for example) mine) ?

Sun, Nov 17, 3:35 AM · efl (efl-1.24), Restricted Project
bu5hm4n added a comment to D10542: Efl.Text.Cursor.

The testcases for test creation via _efl_canvas_text_cursor_create but can a user also create it directly with a the class object ? Is that tested ? What is happening with operations called when no text object is associated? is it verified that this is not crashing, but erroring ? Additionally, is this here tested with ASAN ?

Sun, Nov 17, 3:32 AM · efl
bu5hm4n accepted D10645: Unify "animated" flags.

I am going to merge this on the 20th of November, if there is no voice raised against this.

Sun, Nov 17, 3:13 AM · efl
bu5hm4n added a comment to D10615: introduce efl_canvas_object_animation.

I am going to merge this on the 20th of November, if there is no voice raised against this.

Sun, Nov 17, 3:13 AM · efl
bu5hm4n requested review of D10689: eo: there is no need to count callbacks here.
Sun, Nov 17, 3:11 AM · efl
bu5hm4n added a child revision for D10660: eo: do not over compute the hash when propagating events.: D10689: eo: there is no need to count callbacks here.
Sun, Nov 17, 3:11 AM · efl
bu5hm4n accepted D10660: eo: do not over compute the hash when propagating events..
Sun, Nov 17, 3:06 AM · efl
bu5hm4n accepted D10659: eo: no need to oversize type..
Sun, Nov 17, 3:04 AM · efl
bu5hm4n accepted D10658: eo: refactor shortcut for EFL_EVENT_DESTRUCT event..
Sun, Nov 17, 3:03 AM · efl
bu5hm4n added inline comments to D10660: eo: do not over compute the hash when propagating events..
Sun, Nov 17, 3:02 AM · efl
bu5hm4n accepted D10664: ci/travis: Enable the new build config validation beta feature.
Sun, Nov 17, 2:56 AM · efl
bu5hm4n accepted D10665: ci/travis: remove no longer needed travis keywords.
Sun, Nov 17, 2:56 AM · efl
bu5hm4n requested changes to D10677: elementary: improve focus memory for Efl.Ui.CollectionView..
Sun, Nov 17, 2:56 AM · efl
bu5hm4n added a comment to D10677: elementary: improve focus memory for Efl.Ui.CollectionView..

This looks overall good. However, it took me quite a bit to get the difference of last and previous.

Sun, Nov 17, 2:56 AM · efl
bu5hm4n accepted D10680: ci: remove logic for doing coverity builds only on Saturday.
Sun, Nov 17, 2:44 AM · efl
bu5hm4n accepted D10681: ci: cleanup the check for old meson versions.
Sun, Nov 17, 2:44 AM · efl
bu5hm4n accepted D10685: elementary: reduce event generation during object creation by Efl.Ui.WidgetFactory..
Sun, Nov 17, 2:42 AM · efl
bu5hm4n accepted D10686: elementary: reduce event trigger during object creation stage in Efl.Ui.CollectionView..
Sun, Nov 17, 2:42 AM · efl
bu5hm4n added a comment to D10687: elementary: reduce events triggered by Efl.Ui.PositionManager..

I am wondering here. Doesnt that mean that we are not emitting visibility,changed events anymore ? :/

Sun, Nov 17, 2:41 AM · efl
bu5hm4n accepted D10688: elementary: improve data layout for Efl.Ui.PositionManager*..
Sun, Nov 17, 2:37 AM · efl
bu5hm4n requested changes to D10607: Efl Canvas Text : Modify Style Property.

Hi, additionally to my comments, can you check that all branches of the parsing function is checked and tested (also with asan enabled)? The tests for example do not cover "style=" which has quite a bit of string parsing code. I am also wondering if there is some guideline on what is happening when attributes of the style are not within the normal behavior, is there normally a error ?

Sun, Nov 17, 2:36 AM · efl

Fri, Nov 15

bu5hm4n added a comment to D10662: slider: fix value error from step.

Thank you for the update, however I would still prefer to have the 4 lines refactored into a single function, and not copy and pasted across the same file :)

Fri, Nov 15, 2:48 AM · efl
bu5hm4n added a comment to D10662: slider: fix value error from step.

Oh - i dont mind of the duplicated blocks are in both files. But they should *not* be repeated within the same file ... :)

Fri, Nov 15, 1:14 AM · efl
bu5hm4n requested changes to D10662: slider: fix value error from step.

Okay, i now see what you want to fix. Thank you for the fix & the explanation.

Fri, Nov 15, 1:11 AM · efl
bu5hm4n removed a project from D10615: introduce efl_canvas_object_animation: DO NOT MERGE.
Fri, Nov 15, 12:25 AM · efl

Thu, Nov 14

bu5hm4n added a comment to D10662: slider: fix value error from step.

Can you elaborate in the commit message what exactly is the issue this is fixing ?

Thu, Nov 14, 11:16 PM · efl
bu5hm4n added a comment to D10679: Revert "efl_ui/layout: selectively inhibit theme,changed event".

Does replacing desc == EFL_CANVAS_GROUP_EVENT_MEMBER_ADDED with the a check if the name of the event is "theme,changed" fix the issue ?

Thu, Nov 14, 11:14 PM · efl
bu5hm4n resigned from D10635: Edje : textblock_styles strncmp improvement.

Seems fine. However, i am not a laptop for some time, so someone else needs to land this i guess.

Thu, Nov 14, 1:43 PM · efl
bu5hm4n added inline comments to D10675: efl_ui_multi_selectable: clean this up.
Thu, Nov 14, 1:40 PM · efl
bu5hm4n added a comment to T7871: efl.ui.multi_selectable.

After a little discussion with @cedric on IRC:

Thu, Nov 14, 12:39 PM · efl: api, efl: language bindings
bu5hm4n requested review of D10675: efl_ui_multi_selectable: clean this up.
Thu, Nov 14, 12:20 PM · efl
bu5hm4n added a revision to T7871: efl.ui.multi_selectable: D10675: efl_ui_multi_selectable: clean this up.
Thu, Nov 14, 12:20 PM · efl: api, efl: language bindings
bu5hm4n added a revision to T8265: efl.ui.multi_selectable_async: D10675: efl_ui_multi_selectable: clean this up.
Thu, Nov 14, 12:20 PM · efl (efl-1.24), efl: api, efl: language bindings
bu5hm4n requested review of D10667: remove efl_canvas_animation_player.
Thu, Nov 14, 3:56 AM · efl
bu5hm4n added a child revision for D10666: evas: migrate the vg json example: D10667: remove efl_canvas_animation_player.
Thu, Nov 14, 3:56 AM · efl
bu5hm4n requested review of D10666: evas: migrate the vg json example.
Thu, Nov 14, 3:56 AM · efl
bu5hm4n added a child revision for D10637: efl_ui_spotlight_manager stack: move away from player object: D10666: evas: migrate the vg json example.
Thu, Nov 14, 3:55 AM · efl
bu5hm4n updated the diff for D10637: efl_ui_spotlight_manager stack: move away from player object.

finalize player removal

Thu, Nov 14, 3:55 AM · efl
bu5hm4n updated the diff for D10636: elementary: move away from normal player to the new animation mixin.

finalize player removal

Thu, Nov 14, 3:55 AM · efl

Wed, Nov 13

bu5hm4n accepted D10623: ecore: properly handle children destruction in Efl.BooleanModel..
Wed, Nov 13, 12:05 PM · efl
bu5hm4n accepted D10625: elementary: fix initialization order and memory leak when setting model on Efl.Ui.CollectionView..
Wed, Nov 13, 12:04 PM · efl
bu5hm4n accepted D10634: efl: improve Efl.BooleanModel test by checking the child del case..

Why did that compile for you ? o.O Works for me now.

Wed, Nov 13, 12:02 PM · efl
bu5hm4n added a comment to D10615: introduce efl_canvas_object_animation.

I think this is pretty fine like it is now, there are 2 more things that need to migrate away from player, but that does not block this revision here.

Wed, Nov 13, 12:01 PM · efl
bu5hm4n updated the diff for D10615: introduce efl_canvas_object_animation.

asdfasdf

Wed, Nov 13, 9:08 AM · efl
bu5hm4n updated the diff for D10615: introduce efl_canvas_object_animation.

its efl canvas now.

Wed, Nov 13, 8:31 AM · efl
bu5hm4n added a comment to D10645: Unify "animated" flags.

Okay, i just thought because we prefix here something with animation that is not in our animation mixin ... :) But as i said, i am fine with this :)

Wed, Nov 13, 8:24 AM · efl
bu5hm4n updated the diff for D10637: efl_ui_spotlight_manager stack: move away from player object.

rename

Wed, Nov 13, 7:21 AM · efl
bu5hm4n updated the diff for D10636: elementary: move away from normal player to the new animation mixin.

renames

Wed, Nov 13, 7:20 AM · efl
bu5hm4n updated the diff for D10615: introduce efl_canvas_object_animation.

renames

Wed, Nov 13, 7:20 AM · efl
bu5hm4n edited parent revisions for D10615: introduce efl_canvas_object_animation, added: 1; removed: 1.
Wed, Nov 13, 5:00 AM · efl
bu5hm4n removed a child revision for D10617: efl_canvas_animation: correctly handle double signness: D10615: introduce efl_canvas_object_animation.
Wed, Nov 13, 5:00 AM · efl
bu5hm4n added a child revision for D10645: Unify "animated" flags: D10615: introduce efl_canvas_object_animation.
Wed, Nov 13, 4:59 AM · efl
bu5hm4n added a comment to D10645: Unify "animated" flags.

I am very fine with this.

Wed, Nov 13, 4:59 AM · efl

Mon, Nov 11

bu5hm4n added inline comments to D10615: introduce efl_canvas_object_animation.
Mon, Nov 11, 3:45 AM · efl
bu5hm4n planned changes to D10615: introduce efl_canvas_object_animation.
Mon, Nov 11, 1:54 AM · efl

Sun, Nov 10

bu5hm4n added a comment to D10634: efl: improve Efl.BooleanModel test by checking the child del case..

The assert is not found ...

Sun, Nov 10, 2:15 PM · efl
bu5hm4n requested review of D10637: efl_ui_spotlight_manager stack: move away from player object.
Sun, Nov 10, 2:14 PM · efl
bu5hm4n added a child revision for D10636: elementary: move away from normal player to the new animation mixin: D10637: efl_ui_spotlight_manager stack: move away from player object.
Sun, Nov 10, 2:14 PM · efl
bu5hm4n requested review of D10636: elementary: move away from normal player to the new animation mixin.
Sun, Nov 10, 2:14 PM · efl
bu5hm4n added a child revision for D10615: introduce efl_canvas_object_animation: D10636: elementary: move away from normal player to the new animation mixin.
Sun, Nov 10, 2:14 PM · efl
bu5hm4n updated the diff for D10615: introduce efl_canvas_object_animation.

more tests, and a simple fix

Sun, Nov 10, 2:14 PM · efl
bu5hm4n updated the diff for D10617: efl_canvas_animation: correctly handle double signness.

rebase

Sun, Nov 10, 2:14 PM · efl
bu5hm4n requested changes to D10634: efl: improve Efl.BooleanModel test by checking the child del case..
Sun, Nov 10, 5:04 AM · efl
bu5hm4n added a comment to D10623: ecore: properly handle children destruction in Efl.BooleanModel..

Can you add a test for that ?

Sun, Nov 10, 4:52 AM · efl
bu5hm4n accepted D10624: elementary: fix recursive case during model fetch in Efl.Ui.CollectionView..
Sun, Nov 10, 4:36 AM · efl
bu5hm4n added inline comments to D10625: elementary: fix initialization order and memory leak when setting model on Efl.Ui.CollectionView..
Sun, Nov 10, 4:25 AM · efl
bu5hm4n accepted D10631: ecore: remove the composited children from the source of an Efl.CompositeModel..
Sun, Nov 10, 4:05 AM · efl
bu5hm4n accepted D10632: ecore: correctly apply the offset on the upper part of the boolean mask for Efl.BooleanModel..

Why did this never show up in any test ?

Sun, Nov 10, 4:04 AM · efl
bu5hm4n accepted D10633: efl: remove useless printf from Efl Container Model tests..
Sun, Nov 10, 4:01 AM · efl
bu5hm4n requested changes to D10635: Edje : textblock_styles strncmp improvement.
Sun, Nov 10, 3:57 AM · efl
bu5hm4n updated the diff for D10615: introduce efl_canvas_object_animation.

fix animation. position is now returned *always* from 0.0 to 1.0

Sun, Nov 10, 3:51 AM · efl
bu5hm4n updated the diff for D10617: efl_canvas_animation: correctly handle double signness.

add testcases, and fix more wrong double handling

Sun, Nov 10, 3:48 AM · efl

Fri, Nov 8

bu5hm4n added a comment to T8126: Long term extention for Position_Manager.

Before all that kind of optimization, I think we will need to revise the API of the position manager and its internal a bit. Main issue I am seeing:

  • The collection needs to throttle position manager sizing information request and make sure batch request are not overwhelming the main loop.
Fri, Nov 8, 12:36 PM · Restricted Project, efl: mvvm, efl: widgets

Thu, Nov 7

bu5hm4n requested review of D10628: elm: apply the same fix we needed in entry to other scrollable widgets.
Thu, Nov 7, 11:23 PM · efl
bu5hm4n accepted D10614: elementary: properly handle in flight request in Efl.Ui.CollectionView..
Thu, Nov 7, 1:13 PM · efl
bu5hm4n accepted D10620: eo: make sure to return the right value for efl_event_callback_call..
Thu, Nov 7, 1:08 PM · efl
bu5hm4n added a comment to D10620: eo: make sure to return the right value for efl_event_callback_call..

Nice that you took care of these performance critical whitespace changes! :-D

Thu, Nov 7, 1:07 PM · efl
bu5hm4n accepted D10613: elementary: cleanup Eina_Future properly by relying on efl_future_then proper lifecycle..

lovely.

Thu, Nov 7, 1:05 PM · efl
bu5hm4n added inline comments to D10614: elementary: properly handle in flight request in Efl.Ui.CollectionView..
Thu, Nov 7, 1:02 PM · efl
bu5hm4n requested review of D10622: elm_entry: fix wrong displayed scrollbars.
Thu, Nov 7, 12:49 PM · efl
bu5hm4n added a comment to D10617: efl_canvas_animation: correctly handle double signness.

mhm, then i have to create an entire test suite, as there are no test cases so far :(

Thu, Nov 7, 5:56 AM · efl
bu5hm4n added a comment to T8288: Animation API.

The revision i have linked above now contains pause. You can check the example that got added in this revision and press p for pause (press twice for toggle) or press r for reverse, which will reverse the animation, just as you wished to do that. IMO the code in there look super easy and nice. The speed parameter is essential for the reversing, if i remove it again, the reversing code will be way harder. Would something like the revision be acceptable for you ? If yes, then i would continue removing the player object, as this usage as we have it right now is way easier.

Thu, Nov 7, 5:55 AM · efl: canvas
bu5hm4n updated the diff for D10615: introduce efl_canvas_object_animation.

add a example

Thu, Nov 7, 5:52 AM · efl
bu5hm4n updated the diff for D10617: efl_canvas_animation: correctly handle double signness.

update

Thu, Nov 7, 5:52 AM · efl
bu5hm4n updated the diff for D10615: introduce efl_canvas_object_animation.
Thu, Nov 7, 4:58 AM · efl
bu5hm4n added a child revision for D10617: efl_canvas_animation: correctly handle double signness: D10615: introduce efl_canvas_object_animation.
Thu, Nov 7, 4:58 AM · efl
bu5hm4n requested review of D10617: efl_canvas_animation: correctly handle double signness.
Thu, Nov 7, 4:54 AM · efl