On Thu, 10 Mar 2016 17:41:39 +0100
Armin Krezović <[email protected]> wrote:

> On 10.03.2016 12:37, Pekka Paalanen wrote:
> > On Thu, 10 Mar 2016 01:58:15 +0100
> > Armin Krezović <[email protected]> wrote:
> >   
> >> On 09.03.2016 19:57, Bryce Harrington wrote:  
> >>> Hi Armin,
> >>>
> >>> This is coming along nicely, keep up the good work.  I'm going to follow
> >>> pq's lead here in pointing out more than I usually would, in interest of
> >>> education.  
> > 
> > Thanks Bryce, I essentially agree with everything you said on the
> > commit message.

> > 
> > Everything else in the patch is perfect now. If it wasn't for the
> > clock_format_option memory leak, I would just push this patch upstream
> > after testing it. All the other complaints are very minor. There is
> > also one hunk that doesn't really belong in this patch. ;-)
> >   
> 
> I suppose you're talking about the whitespace fix? I can revert that
> part in the whole if necessary. I've just accidentaly fixed that when
> I played with getting the widget size from text extents.

That was the one, but I also think it's dead code, as nothing is using
the extents received. And it's not quite the right place for it either.

> > With the leak fixed, this is:
> > Reviewed-by: Pekka Paalanen <[email protected]>
> > 
> > (The above sentence means that you can just add my R-b line in your
> > next version of this patch below your S-o-b line, if you made exactly
> > the changes I asked for and nothing else.)
> > 
> > 
> > Thanks,
> > pq
> >   
> 
> Thank you once more for taking your time to review and give advices.

I see your v4 has landed. :-)


Thanks,
pq

Attachment: pgpX4GeZyGYjo.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to