[chirp_devel] FT70 Driver - Issue #5329

[Dan Smith]

> -    } digital_settings;
> +    } digital_settings;   

You're adding trailing whitespace here.

> @@ -428,7 +390,6 @@
>          _bank = self._model._radio._memobj.bank_info[self.index]
>          _bank.name = [ord(x) for x in name.ljust(6, chr(0xFF))[:6]]
>  
> -
>  class FT70BankModel(chirp_common.BankModel):

Can you avoid random whitespace damage where ever possible? It makes it much easier to track changes to actual patches in the future. Also, PEP8 specifies two blank lines between top-level items, so this is making it less correct. Not that chirp is a shining beacon of PEP8 compatibility, but... :)

> @@ -704,15 +619,15 @@
>          mem.skip = flag.pskip and "P" or flag.skip and "S" or ""
>          mem.name = self._decode_label(_mem)
>  
> -        mem.extra = RadioSettingGroup("extra", "Extra Settings")
> -
> -        rs = RadioSetting("display_tag", "Display Name/Frequency",
> -                          RadioSettingValueBoolean(_mem.display_tag))
> -        mem.extra.append(rs)
> -
> -        rs = RadioSetting("ams", "AMS",
> -                          RadioSettingValueBoolean(_mem.ams))
> -        mem.extra.append(rs)
> +        # mem.extra = RadioSettingGroup("extra", "Extra Settings")
> +        #
> +        # rs = RadioSetting("display_tag", "Display Name/Frequency",
> +        #                   RadioSettingValueBoolean(_mem.display_tag))
> +        # mem.extra.append(rs)
> +        #
> +        # rs = RadioSetting("ams", "AMS",
> +        #                   RadioSettingValueBoolean(_mem.ams))
> +        # mem.extra.append(rs)

Can you just remove code you don't want to be present? Commented-out code is distracting to read while debugging. Git will remember what it looked like if you want to add it back later :)

Also, if you can send the patches inline (without whitespace damage) it's really appreciated as it makes reviewing them much quicker. You might check out the patchbomb instructions on the dev pages, as it makes sending these emails hg itself trivial.

Thanks!

--Dan