> On 16 Oct 2015 16:01, Brian Norris wrote:
> > On Fri, Oct 16, 2015 at 10:49:04PM +0000, Finucane, Stephen wrote:
> > > > Looks good to me, though I'm still a little green on Python 2/3
> > > > compatibility. I've been runnning this (plus other patches) since you
> > >
> > > What are your thoughts on the use of 'six' for this stuff? Would it
> help you here?
> >
> > I'm not very familiar, but before reading this email I was trying to
> > figure out the "best" interoperable way to replace dict.iteritems(), and
> > I see we could use six.iteritems(). So my initial review says "sure"!
> 
> dict.items() should work fine in both.  the question becomes whether the
> dict
> is so large that coercing it to a list first adds significant overhead.
> i've
> found that generally it doesn't.

`list(dict.items())` to keep it compatible with Python 3, right? It's safe to 
say that exceptionally few developers are going to have machines with <512MB 
RAM today: we're good here.

> > Except then, we rely on PyPy and nonstandard libraries which doesn't
> > seem too good to me for such a small utility :(
> 
> six is a pretty common module.  i wouldn't worry about requiring it.
> -mike

Nah, I think he's right here. At least for the server it's packaged with Django 
but for users downloading pwclient from the server all they get is a single 
executable file. It's another barrier to entry if they need to install six 
before they can use it. I'm going to investigate (emphasis on investigate haha) 
the possibility of moving pwclient out of tree so it might be an option then 
but otherwise I guess we should stick to the hard 'if VERSION' checks, ugly 
though they may be.

Stephen
_______________________________________________
Patchwork mailing list
[email protected]
https://lists.ozlabs.org/listinfo/patchwork

Reply via email to