Patch Set 3: (2 comments)
https://gerrit.osmocom.org/#/c/7160/3/src/osmo_gsm_tester/bts.py File src/osmo_gsm_tester/bts.py: Line 26: class Bts(log.Origin, metaclass=ABCMeta): > could you explain ABCMeta? What does it help us with? https://docs.python.org/3/library/abc.html It helps us specifying in the code a more clear inheritance structure, which becomes important as we add more and more BTS types. For instance, if a method is marked as abstract and you try to instantiate a class which doesn't override the method, python will fail with a clear error. This is good in case we want to change some API, this way we make sure all subclasses are also updated. It also helps to quickly spot abstract methods in a base class and then known which ones I need to implement when creating a new subclass. Line 45: # PUBLIC - INTERNAL API > what now, public or internal? I don't really like these huge comments, I gu The info can get into the doc, but having these blocks clealry helps me find quickly which methods of the class are expected to be available in each context. The comment block states the methods in there are public, since we want to call them from outside the class, but still they are for now not intended for use by tests, only by our internal classes. -- To view, visit https://gerrit.osmocom.org/7160 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic13133e61abda73a8b507c1a1bd7b98c677460f9 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-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-HasComments: Yes
