Developer tools #26
Developer tools #26
Conversation
|
Rád bych povolil actions ale nějak mě to nechce vyskočit (dle dokumentace) |
|
je tam potřeba přidat že je to možné to rozběhnout na forku, mělo by to být toto? nebo pull_request:? |
|
Upravil jsem ten reusable workflow aby uměl vlastní matrix. Syntaxi jsem našel v docs, měla by vypadat takto strategy:
matrix:
php-version: ["7.4", "8.0", "8.1", "8.2"]
uses: wrk-flow/reusable-workflows/.github/workflows/php-check.yml@main
with:
phpVersion: ${{ matrix.php-version }}Ještě mě napadlo jestli nelocknout work-flow dle commitu. |
|
Vyladil jsem to na jiném repu, toto by mohlo fungovat dobře. Rozdělil jsem testy oproti static analysis. Poběží to i na pull requestech. Myslím, že ecs / rector / phpstan asi nechceme, aby běželi na všech php verzích kde to testujeme. phpVersion flag je možný poslat. Co myslíš? |
|
ECS, rector, PHPStan mají smysl jen na nejvyšší verzi. Na všech verzích bych pouštěl jen testy |
MartinMystikJonas
force-pushed
the
dx
branch
3 times, most recently
from
274a47f to
3b06966
Compare
|
Nahodil sme novou verzi s úpravami |
|
Hmm, ještě jsem nezkoušel github actions na pull requestech od jiných a očividně ani to
|
|
koukal sme na pár dalších projektů a pull_request by mělo stačit. Zkusil sme to teď ještě opravit podle toho co máme v phpstan-latte, kde předáváme ještě parametr null. Jinak to bude spíš asi o nastavení repository |
|
Zkus zkouknout jestli je to povolené u https://github.com/pionl/smart-emailing-v3/settsing/actions |
|
Případně je otázka jestli si to nepamatuje stav, kdy byl PR vytvořený. Zkusil sem ho poslat znovu |
|
No a nebo je možnost, že to záleží na configu v masteru - což by dávalo smysl, protože jinak by PR mohlo obejít omezení co nastavil maintainer. |
|
Je to povolené. Mám pocit, že to opravdu bude tím masterm. Hodil jsem tam action. Zkus prosím rebase.
|
|
Jo už to čeká na approval |
|
se tam našel pěkný bug na lowest dependency |
|
No Guzzle 6 už asi nemá cenu podporvat a řešit kompatibilitu se 7. Dám tam jako minimum 7. |
|
Případně předělat na PSR interface ale to už by bylo si myslím zbytečně moc "předělávání". |
|
Opravil sem PHPStan chyby a projel to coding standardem, rectorem. Mělo by to být raedy na merge pokud projde workflow. Pak se teda asi pustím do tech změn struktury. Všiml sem si ještě jendé věci - dost často se tam přepisuje metoda jen proto, aby se změnil typ parametr/návratové hodnoty - asi bych to přepsal na generiky |
| * | ||
| * @return static | ||
| */ | ||
| public static function fromJSON($json) | ||
| { | ||
| $item = new static(); | ||
| $item = new static(); /** @phpstan-ignore-line */ |
There was a problem hiding this comment.
nedáme __construct v modelu jako final a tím pádem to bude správně?
There was a problem hiding this comment.
To bysme mohli
There was a problem hiding this comment.
Tak nemohli. Některé Modely redefinují konstruktor.
There was a problem hiding this comment.
Tak to pak ta funjkce nemůže fungovat správně. Kouknul jsem užití a nikdy fromJson se nepřebijí.
Co udělat toto:
- tam kde se funkce fromJson používá tak udělat novou do které dáme: final construct + fromJson.
- ta bude extendovat Model
- bude se jmenovat třeba jako
FromJsonModel - tak kde funkce byla použita tak použijeme místo třídy
ModelpoužijemeFromJsonModel
Hm? :)
|
Tak opraveno. Muzes to mergnout |
|
Díky! |
No description provided.