[poppler] Releasing RC1 one week later

[Brad Hards]

On Saturday 08 March 2008 08:29:03 pm Carlos Garcia Campos wrote:
> El vie, 07-03-2008 a las 20:03 +1100, Brad Hards escribió:
> > On Monday 03 March 2008 09:25:10 pm Carlos Garcia Campos wrote:
> > > I've started to look at the optional content stuff and I've realized
> > > there are some things  implemented in the qt4 frontend that should be
> > > in the core, like the order array and radio buttons stuff. I've
> > > changed/fixed other minor issues. Bradh, could you have a look at these
> > > patches, please?
> > >
> > > http://people.freedesktop.org/~carlosgc/poppler-optcontent.diff
> >
> > I haven't tried / tested any of them, but I'll still offer my opinion,
> > based on initial review.
>
> we can just keep both interfaces, getOCGs() returns the
> optionalContentGroups and getOrderArray() the array object, and
> getOrderedOCGs or something like that would return the list of optional
> content already ordered.
Please don't do this. Just use the existing interface. Then people who don't 
want to present the UI part don't pay for parsing.
If you do both, then the original argument for common code doesn't hold 
anyway.

> > The foo() -> getFoo() rename is fine (not standard for Qt, but looks more
> > standard for poppler),
>
> yes, that's consistent with poppler core code.
>
> >  but please don't put the implementation in the header.
>
> ok, no problem, but just out of curiosity, why?
Because it limits ability to fix things later, and because it is ugly. I 
shouldn't have done any, but almost certainly did. It isn't that big a deal.

> > From: Carlos Garcia Campos <carlosgc at gnome.org>
> > Date: Sun, 2 Mar 2008 19:43:15 +0100
> > Subject: [PATCH] Use an array instead of a list for optional content
> > groups Sorry, I don't understand what you are trying to accomplish with
> > this. Seems gratuitous. Can you explain more?
>
> We don't really need a list when we already know the size of the list.
> An array it's much more efficient than a list and it's also more
> consistent with the poppler code.
Did you do any measurements of the impact?
I really don't care to much, but make sure you don't break the Qt4 bindings.

> > >From 157d763c3e0e5432cce4da391fab99d0854a63f5 Mon Sep 17 00:00:00 2001
> >
> > From: Carlos Garcia Campos <carlosgc at gnome.org>
> > Date: Sun, 2 Mar 2008 19:52:37 +0100
> > Subject: [PATCH] Fix memory leaks
> > I don't see the leaks. Again, can you explain why this change is needed?
>
> m_orderArray and m_rBGroupsArray are never freed. By using objects
> instead of pointers you avoid allocations/deallocations too.
Seems like an excessive change. Why not just free it in the destructor?