Re: [E-devel] elm_calendar_ first_day_of_week API

[Bruno Dilly]

Hey Michael,

really nice feature! Good stuff. Thank you for your patch.

A few comments after a quick review:
- when you've moved code to _fill_weekdays, var "weekday" declaration
was left behind.
- I don't think it will work fine when used together to
weedays_names_set (please check this, but your patch doesn't look to
be handling that).
- Also, please, try to cover your new API on calendar tests (I was
trying to leave everything covered)

So I suggest you to make these changes before committing go svn.

Best regards

On Wed, Mar 28, 2012 at 10:57 AM, michael bouchaud
<michael....@gmail.com> wrote:
> Hi, everybody. We are near to release elementary. But I want to push one
> patch in
> more, who add 2 API. The first one is to set the first day of week in
> elm_calendar
> widget and second one to get the info. Please review it, but don't commit
> it.
> I will push it, if you agreed.
>
> --
> Michaël Bouchaud
>
> ------------------------------------------------------------------------------
> This SF email is sponsosred by:
> Try Windows Azure free for 90 days Click Here
> http://p.sf.net/sfu/sfd2d-msazure
> _______________________________________________
> enlightenment-devel mailing list
> enlighten...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>

--
Bruno Dilly
Senior Developer
ProFUSION embedded systems
http://profusion.mobi

------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
enlightenment-devel mailing list
enlighten...@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

[michael bouchaud]

2012/3/29 Bruno Dilly <bdi...@profusion.mobi>

> Hey Michael,
>
> really nice feature! Good stuff. Thank you for your patch.
>
> A few comments after a quick review:
> - when you've moved code to _fill_weekdays, var "weekday" declaration
> was left behind.
>

Yes I see it after send it. Removed :)

> - I don't think it will work fine when used together to
> weedays_names_set (please check this, but your patch doesn't look to
> be handling that).
>

ok, will test it.

> - Also, please, try to cover your new API on calendar tests (I was
> trying to leave everything covered)
>

Sure :)

>
> So I suggest you to make these changes before committing go svn.
>
> Best regards
>

Thx for your review ! :)
Regards