Dear Kim Shinwoo, thanks for your revised patch. I also have some comments.
1. actionslider Wrong indentation again. Do not use tab. I fixed it. Printing enabled position is way too complicated. I simplified it. This is in SVN! Thanks anyway :) 2. box You didn't remove unnecessary blank line. It's in line 290 in your patch. Wrong convention. EFL does not use space for casting. wrong> box = (Evas_Object *) data right> box = (Evas_Object *)data wrong> rl = (Eina_List *) evas_ob.... right> rl = (Eina_List *)evas_ob Add more (, ) for if conditions. > (pos == BOX_PACK_POSITION_START || pos == BOX_PACK_POSITION_END) >> ((pos == BOX_PACK_POSITION_START) || (pos == BOX_PACK_POSITION_END)) And check other wrong formatting. You missed test.c diffs. And functions are too much complicated than needed. This can be enhanced later. So I'll not list up all the things now. 3. ecore_evas_x I think it would be better if you create a separate mail thread for this. This patch covers different boundaries with previous mail. Anyhow, I have no idea how _ecore_evas_x_title_set works but previous code was setting icccm_title and netwm_name even parameter 't' was NULL. Is that ok to skip that setting when 't' is NULL? Actually there is no NULL check code in ecore_evas_x, I don't know if it's intended or not. Sometimes it's intended for the lower level code due to the performance. I think elm_win_title_set() may filter NULL instead. Can anybody else review this patch? Anyway, thanks for the contribution. Daniel Juyung Seo (SeoZ) On Thu, Feb 16, 2012 at 3:22 PM, Kim Shinwoo <kimcinoo....@gmail.com> wrote: > > Dear Daniel, Thanks! > > I have attached patch based on your comment. > > And more.. > elm_win_title_set(win, 0); makes CRASH. > The last patch is for this. Please review this also and give any feedbacks. > > Sincerely, > Shinwoo Kim. > > 2012/2/16 Daniel Juyung Seo <seojuyu...@gmail.com> > > > Thank you very much cnook. > > I have some brief comments here. > > > > 1. actionslider > > 'switch-case' indentation is wrong. Please check below e-coding style page. > > http://trac.enlightenment.org/e/wiki/ECoding > > And for elm_actionslider_enabled_pos_get, how about printing it like > > below? > > "actionslider enabled pos: left, right" > > > > 2. bg > > Trailing white space in line 14. > > But this patch looks ok. I removed the trailing white space. In SVN! > > > > 3. box > > Use elm_win_util_standard_add() instead of elm_win_add + elm_bg_add for > > convenience. > > elm_scroller_bounce_set(sc, 0, 1); -> Use EINA_FALSE, EINA_TRUE for > > Eina_Bool for consistency. > > Fix formatting. > > Remove unnecessary blank line. > > > > evas_object_size_hint_min_set(bg, 160, 160); > > evas_object_size_hint_max_set(bg, 640, 640); > > -> really needed? > > > > Daniel Juyung Seo (SeoZ) > > > > On Thu, Feb 16, 2012 at 10:20 AM, cnook <kimci...@gmail.com> wrote: > > > > > Dear All, Hello~ > > > > > > As you guess, the patch is for the elementary_test. > > > Please review the patch and give any feedbacks. > > > > > > The test_actionslider patch is mainly for elm_actionslider_xxx_get() > > APIs. > > > It would not be a good idea to add test code for such APIs. > > > So I have planed to add test code except elm_xxx_get() APIs > > > > > > Sincerely, > > > Shinwoo Kim. > > > > > > > > > > > ------------------------------------------------------------------------------ > > > Virtualization & Cloud Management Using Capacity Planning > > > Cloud computing makes use of virtualization - but cloud computing > > > also focuses on allowing computing to be delivered as a service. > > > http://www.accelacomm.com/jaw/sfnl/114/51521223/ > > > _______________________________________________ > > > enlightenment-devel mailing list > > > enlightenment-devel@lists.sourceforge.net > > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > > > > > > > ------------------------------------------------------------------------------ > > Virtualization & Cloud Management Using Capacity Planning > > Cloud computing makes use of virtualization - but cloud computing > > also focuses on allowing computing to be delivered as a service. > > http://www.accelacomm.com/jaw/sfnl/114/51521223/ > > _______________________________________________ > > enlightenment-devel mailing list > > enlightenment-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > ------------------------------------------------------------------------------ > Virtualization & Cloud Management Using Capacity Planning > Cloud computing makes use of virtualization - but cloud computing > also focuses on allowing computing to be delivered as a service. > http://www.accelacomm.com/jaw/sfnl/114/51521223/ > _______________________________________________ > enlightenment-devel mailing list > enlightenment-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > ------------------------------------------------------------------------------ Virtualization & Cloud Management Using Capacity Planning Cloud computing makes use of virtualization - but cloud computing also focuses on allowing computing to be delivered as a service. http://www.accelacomm.com/jaw/sfnl/114/51521223/ _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
