Attribute #70
Attribute #70
Conversation
| private function findAllClasses(Finder $finder, CacheMeServiceContract $cacheMeService): array | ||
| { | ||
| $cacheKey = 'larastrict.make.expectation.interfaces'; | ||
| $cachedInterfaces = $cacheMeService->get($cacheKey, static fn (): ?array => null); |
There was a problem hiding this comment.
Použil jsem keš, pro další průchod, se neparsují již existující soubory.
There was a problem hiding this comment.
Tu keš asi nechápu :) Ty chceš to cachovat mezi volání commandu? Nevím jestli je to správně pokud tam nebude stejně check jestli implementace se nezměnila, a to mě připadá trochu out of scope (docela dost pracné).
Jinak za mě špatně použitá ta funkce - přesunout "ten" search do fn () a bud se spustí pokud cache není a nebo ne.
|
|
||
| interface SetUpFinderActionContract | ||
| { | ||
| public function execute(Finder $finder): Finder; |
There was a problem hiding this comment.
Říkal jsem si jestli udělat factory na Finder, nebo jen takto mít možnost upravit?
There was a problem hiding this comment.
Dal bych factory jak píšeš a consumer si to pak přes container můžeš "switchnout"
| $class = (string) $this->input->getArgument('class'); | ||
|
|
||
| if ($class === 'all') { | ||
| $finder = Finder::create()->files()->name('*.php'); |
There was a problem hiding this comment.
Hmm asi bych udělal factory které mě sestaví ten finder místo setup, připadá mě to čistější a lépe testovatelné stubem
| private function findAllClasses(Finder $finder, CacheMeServiceContract $cacheMeService): array | ||
| { | ||
| $cacheKey = 'larastrict.make.expectation.interfaces'; | ||
| $cachedInterfaces = $cacheMeService->get($cacheKey, static fn (): ?array => null); |
There was a problem hiding this comment.
Tu keš asi nechápu :) Ty chceš to cachovat mezi volání commandu? Nevím jestli je to správně pokud tam nebude stejně check jestli implementace se nezměnila, a to mě připadá trochu out of scope (docela dost pracné).
Jinak za mě špatně použitá ta funkce - přesunout "ten" search do fn () a bud se spustí pokud cache není a nebo ne.
|
|
||
| interface SetUpFinderActionContract | ||
| { | ||
| public function execute(Finder $finder): Finder; |
There was a problem hiding this comment.
Dal bych factory jak píšeš a consumer si to pak přes container můžeš "switchnout"
| use Attribute; | ||
|
|
||
| #[Attribute] | ||
| final class TestAssert |
There was a problem hiding this comment.
jakože Testing/Attributes/Tests ?
tests/ jsou testy, tam asi nebudu dávat atribut.
There was a problem hiding this comment.
Ne myslím tím tento objekt TestAssert který očividně slouží pro náš test :)
h4kuna
force-pushed
the
attribute
branch
3 times, most recently
from
1d7492e
to
1e63dab
Compare
|
Seems like all working as expected! Can you finalize commit message? |
| // our implementation expets always composer.json file | ||
| $this->fileSystem->shouldReceive('isFile') | ||
| ->once() | ||
| ->with(function($arg) use ($expectedComposerCheck){ |
There was a problem hiding this comment.
usage is withArgs and method must return boolean
| use Attribute; | ||
|
|
||
| #[Attribute] | ||
| final class TestAssert |
| public function __construct( | ||
| private readonly Filesystem $filesystem, | ||
| private readonly ComposerJsonDataService $getComposerJsonDataAction, |
There was a problem hiding this comment.
$getComposerJsonDataAction -> $composerJsonDataService
| private array $dirs = []; | ||
|
|
||
| public function __construct( | ||
| private readonly ComposerJsonDataService $getComposerJsonDataAction, |
There was a problem hiding this comment.
$getComposerJsonDataAction -> $composerJsonDataService
Můžeš na to mrknout.