[chirp_devel] Adding Settings support to the VX8-R

Dan Smith
Mon Jun 5 11:36:15 PDT 2017


> The patch is somewhat lengthy due to the fact that I promoted most of
> the VX8DRadio class functions to the VX8Radio base class. This left the
> VX8DRadio class with just functions that were
> somewhat different for the VX-8DR and VX-8GE (e.g.
> _get_aprs_tx_settings) or unique to the VX-8DR and VX-8GE (e.g.
> _get_aprs_smartbeacon).
> 
> Hopefully, I haven't introduced any PEP-8 violations..PyCharm is pretty
> good at yelling at me about those. Please review and feel free to toss
> the whole thing if it's felt that I'm off base
> on this approach.

I'm okay with the refactor (in fact, I'm sure it's sorely needed), but
do you think you could split this up into smaller pieces? Ideally, you'd
lead with a refactor of things up into the base class, and then follow
on with the "new" stuff in a separate patch. If we merge and release
this, and then in six months someone reports a weird regression, it'll
be hard to do anything other than revert this entire patch to see if it
solves the problem.

I know it'll be some work, but... are you willing?

Thanks!

--Dan



More information about the chirp_devel mailing list