Closed gromgull closed 5 years ago
Mye av det er jo frivillig da. Det er bare pakke-greia dere må fixe, ellers kan ikke folk bruke SDKen.
Naming i python er ofte ganske lax, mine forslag er fra https://www.python.org/dev/peps/pep-0008/#naming-conventions, men som det også vel står der så finnes det viktigere ting enn naming, så bare la det være. Men lett/billig å fixe det nå da, i forhold til om et år!
Skjønner hva du mener, men naming er jo viktig. Det er også viktig at SDK-en føles python-native, hvis ikke så kunne vi latt være og heller bare gitt ut swagger for REST API-et. Poenget er at det skal være så plug&play for utviklerne i SDK-ene at det er en no-brainer å bruke. Så vi må fikse det nå 😉
ikke ha
-
i package name. Enten_
eller baretarget365
? (import target-365
er syntax error)dere er litt forvirret mtp. pakker, dvs. flytt alt inn i target365 pakka - dere kan bruke relative imports inni de andre filene, dvs.
from models.keyword import Keyword
blirfrom .models.keywords import Keyword
apiClient
fila bør heteapi_client
Metoder er vanligvis i snakecase, dvs.
get_all_keywords
, ikkeGetAllKeywords
Alle variabler bør også være snakecase, hvis ikke de matcher akkurat det som kommer ut av APIet og dere vil ha consistency på alle versjoner av SDKen. Men det er mindre viktig enn metoder.
hvis det bare er noen få hoved-klasser som folk bruker, dvs.
ApiClient
sikkert, så import den inn i__init__.py
, så kan folk bare gjørefrom target365 import ApiClient
. Samme med models, du kan importe alt fra Keywords osv. inn i init der, så blir detfrom models import Keyword
HttpError
er allerede definert i requests, bare bruk den? Eller lag en egenTarget365Exception
I models er det fint med objekter, med
fromDict
er veldig upytonisk. Bruk bare__init__(**kwargs)
, har du en dict kan du sende den inn medKeyword(**min_dict)
denne koden gjør litt vondt: https://github.com/Target365/sdk-for-python/blob/master/target365-sdk/models/keyword.py#L16-L22 :D Det burde være:
return [ Keyword( **d ) for d in listOfKeywords ]
dere kan også la alle modellene inherite fra en abstract model, og legge "fromList" funksjonen der.
Hvis dere er happy med å bare støtte python 3.6/3.7, så kan dere se på dataclasses ( https://docs.python.org/3/library/dataclasses.html / https://github.com/ericvsmith/dataclasses )
Men MANGE bruker python 2 enda ... :D
Har ikke så mye med python å gjøre, men legg til eksempel bruk i readme og et eller flere ferdige eksempler?
Når det er "ferdig" - push releaser til pypi så folk kan installere med bare
pip install target365