Changed and unified structure (BC breaks)

pionl / smart-emailing-v3

Wrapper for SmartEmailing API v3 version in PHP

MIT License

13 stars 14 forks source link

Changed and unified structure (BC breaks) #28

Closed MartinMystikJonas closed 1 year ago

MartinMystikJonas commented 1 year ago

Zkus to prosím zkouknout. JE tam hodně změn takže changelog je dost matoucí. Možná by bylo lepší zkouknout výslednou strukturu přímo.

Pokud to bude ok opravím a doplním ještě readme.

MartinMystikJonas commented 1 year ago

Změny ve stručnosti:

všechny modely přesunuté do Models, některé z nihc přejemnované kvůli konfliktům názvů
namespace Requsts -> Endpoints
sjednocená struktura Endpointů a Requestů
Requesty mají generiky určující jakou vrací Response
Response mají generiku určující jaká vrací data
abstrakce obvyklých typů response StatusResponse, CollectionResponse, ItemResponse
odstraněné některé duplikace kódu
nějaké drobné úpravy a odstranění zbytečného kódu všude možně

pionl commented 1 year ago

Můžeš prosím tě přidat UPGRADE.md a kapitolu 0.4.1 -> 1.0.0 ?

A vypsat tam cca postup jak to "rozumně upgradovat" ze staré verze na novou?

MartinMystikJonas commented 1 year ago

Doplněno a upravil sme i README. Ještě to bude asi chtít trochu učesat než to mergneme, ale už bys to mohl zkouknout jestli je to za tebe ok takhle.

pionl commented 1 year ago

Určitě podívám, mám to tady otevřené a postupně na to koukám :) Dnes večer to budeš mít

pionl commented 1 year ago

@stanislav-janu chceš se na změny podívat?

MartinMystikJonas commented 1 year ago

Typehintu tam bude chybet vic. Planuju pozdeji prejit na PHPStan level 8 a vsude je doplnit.

stanislav-janu commented 1 year ago

@stanislav-janu chceš se na změny podívat?

Krom typehintů a pár věcí, co jsem poslal jako line comment tam z mého pohledu nevidím žádný zádrhel. Chce to ještě vyzkoušet v reálu. Koukám jen, že nekteré testy jsou skipnuty. Víme proč?

stanislav-janu commented 1 year ago

Nechceme používat konstanty jako CamelCase? Tak jako na to přešlo Nette? Uppercase je docela přežitek. Ale je to jen kosmetika. A nepreferuji mezeru za vykřičníkem. Preferuji zápis if (!$param). Taky jen kosmetika k zamyšlení.

MartinMystikJonas commented 1 year ago

Skipnute jsou testy, ktere by realne zkousely sahat na live API. Pri testovani je mozne skipnuti zakomentovat a vyzkouset jak to funguje.

Ohledne nazvu konstant je mi to v zasade jedno. Pomud prederujete CamelCase muzu predelat.

Osttani kosmeticke veci veci jsou defaultni ECS - nevim jestli ma cenu ho prenastavovat.

MartinMystikJonas commented 1 year ago

Mělo by to but ready na merge. Změnu conding standardu kdyžtak můžem udělat později.

MartinMystikJonas commented 1 year ago

Je ode mě ještě něco potřeba aby se to mergnulo?

MartinMystikJonas commented 1 year ago

@pionl Muzes tohle prosim mergnout? Mam ready navazujici PR, který přidává typy. A pak bych chtěl dodělat ty endpointy co mi chybí.

pionl commented 1 year ago

všem se omlouvám, měl jsem ted celkem mazec.

Takto je to super. Za mě constanty UpperCase, kde je možné enum (když už se to toho takto šahá :)

A díky moc @MartinMystikJonas za skvělý příspěvek do projektu (a věřím že i oživení).

@stanislav-janu díky za review!