This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Max gerrit-no-reply at lists.osmocom.orgPatch Set 3: (7 comments) The rest should be fixed in new version. https://gerrit.osmocom.org/#/c/1265/3/openbsc/contrib/ipa.py File openbsc/contrib/ipa.py: Line 27: class IPA(object): > IMvHO despite being an acronym, the class name should avoid all-caps I think it's better to stick to the same names as in libosmocore. Line 35: PROTO = dict(RSL = 0x00, CCM = 0xFE, SCCP = 0xFD, OML = 0xFF, OSMO = 0xEE, MGCP_OLD = 0xFC) > typically in named args to function calls, there are no spaces around the ' This would decrease readability without providing any obvious benefits. Line 45: """ > A single line docstring like this could stay on a single line, especially i I think it's better to use the same style for docstrings regardless of how many lies they consist of right now. Line 48: return 'UNKNOWN' > I think better would be to return None or raise an exception. This would make dispatcher code which uses it unnecessary hard without providing any obvious benefits. Line 50: def __tag(self, t, v): > any particular reason for *double* underscore? that's python syntax for private functions. Line 113: return self.__tag(self.IDTAG['SERNR'], data) > I think it would be good to define the IDTAG keys as constants, like the TC Calling code is expected to work with tag_* functions. Line 163: def identity(self, unit = b'', mac = b'', location = b'', utype = b'', equip = b'', sw = b'', name = b'', serial = b''): > here I'd also drop the spaces around '='. why? it's easier to read when spaces are there. -- To view, visit https://gerrit.osmocom.org/1265 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41e37ec143183e422c0b31af95d183bd84f0c328 Gerrit-PatchSet: 3 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Max <msuraev at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Max <msuraev at sysmocom.de> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-HasComments: Yes