Patch Set 3: Code-Review+2

(4 comments)

this is just peanuts comments, feel free to decide yourself

https://gerrit.osmocom.org/#/c/7167/3/src/osmo_gsm_tester/powersupply.py
File src/osmo_gsm_tester/powersupply.py:

Line 23: class PowerSupply(log.Origin, metaclass=ABCMeta):
what's the rationale for the meta class to inherit from? It's enough to just 
have the PowerSupplySispm in loosely typed python? AFAICT the parent class 
doesn't serve any purpose. Are we planning to add more KNOWN_SUPPLY_TYPES any 
time soon? Otherwise, I guess I'd just have the Sispm and be done ==  about 
half the amount of overhead code.


Line 42:         'Turn on (onoff=True) or off (onof=False) the device'
typo "onof"


https://gerrit.osmocom.org/#/c/7167/3/src/osmo_gsm_tester/powersupply_sispm.py
File src/osmo_gsm_tester/powersupply_sispm.py:

Line 61:     def powered(self):
would prefer "is_powered()", or "power_get()" to match below


Line 66:         'Turn on (onoff=True) or off (onof=False) the device'
"onof"


-- 
To view, visit https://gerrit.osmocom.org/7167
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I981c260eca1a61657147e6d83b4226618088223c
Gerrit-PatchSet: 3
Gerrit-Project: osmo-gsm-tester
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-HasComments: Yes

Reply via email to