Review Request: wetter.com Weather Ion

[Marco Martin]

On Tuesday 03 November 2009, Thilo-Alexander Ginkel wrote:
> On Tuesday 03 November 2009 18:51:05 Aaron Seigo wrote:
> > > On 2009-11-03 17:00:51, Will Stephenson wrote:
> > > > /trunk/kdereview/plasma/dataengines/weather/ions/ion_wettercom.cpp,
> > > > line 44
> > > > <http://reviewboard.kde.org/r/2026/diff/2/?file=13614#file13614line44
> > > >>
> > > >
> > > >     By convention, public member variables of pimpl classes are not
> > > > names with m_ prefixes - since accessing them via d-> in the outer
> > > > class makes it obvious that they belong  to the pimpl
> >
> > this isn't public API. why does it have a pimpl at all? i think some
> > people have missed the point of the FooPrivate classes: maintaining
> > binary compatibility in publicly exported APIs. for anything else they
> > are just a bit more overhead, one more level of indirection and one more
> > opportunity for a memory leak.
> 
> I chose this approach as that's what the existing Weather Ions are using,
>  but also agree that this is not a public API, so binary compatibility is a
>  non- issue. If you prefer, I can easily remove the pimpl.

yes please :)

> Regards,
> Thilo
> _______________________________________________
> Plasma-devel mailing list
> Plasma-devel at kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
> 


-- 
Marco Martin