Re: [PATCH 2/2] wireless-regdb: Parse wmm rule data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, May 01, 2018 at 11:19:21PM +0200, Johannes Berg wrote:
> On Tue, 2018-05-01 at 15:02 -0500, Seth Forshee wrote:
> > 
> > > +import attr
> > 
> > I'm a little hesitant to add use of non-standard libraries if it isn't
> > necessary, as some distros have traditionally built the regdb from
> > source (not sure how practical that is after the db-as-firmware changes
> > though). Do we lose anything critical if we don't use attr?
> 
> I probably suggested the use of attr. It's super useful, for things like
> this:
> 
> > > +@attr.s(frozen=True)
> > > +class WmmRule(object):
> > > +    vo_c = attr.ib()
> > > +    vi_c = attr.ib()
> > > +    be_c = attr.ib()
> > > +    bk_c = attr.ib()
> > > +    vo_ap = attr.ib()
> > > +    vi_ap = attr.ib()
> > > +    be_ap = attr.ib()
> > > +    bk_ap = attr.ib()
> > > +
> > > +    def _as_tuple(self):
> > > +        return (self.vo_c, self.vi_c, self.be_c, self.bk_c,
> > > +                self.vo_ap, self.vi_ap, self.be_ap, self.bk_ap)
> 
> Spelling this out as a real object with all the __repr__ and comparisons
> etc. gets far more verbose.
> 
> While we can get rid of it in theory, it's a ~100KiB package without any
> further dependencies, and the code is better off for it.
> 
> Ultimately it's your decision, but I suspect that python is already such
> a big dependency that adding this library is in the noise.

I'm more thinking that for e.g. Debian if attr is installed via pip and
not a distro package then that might be a problem for their package
builds. But let's go ahead and leave it, if there's fallout we can
figure that out later.

Thanks,
Seth

_______________________________________________
wireless-regdb mailing list
wireless-regdb@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/wireless-regdb



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux