Update check system #1189
Update check system #1189
Conversation
jbrooksuk
force-pushed
the
update-system
branch
from
42289c0
to
6c85cfa
Compare
|
Ping @cachethq/owners |
jbrooksuk
force-pushed
the
update-system
branch
from
6c85cfa
to
b5b60ff
Compare
|
Very nice James! 😍 |
| * | ||
| * @return \Illuminate\Http\JsonResponse | ||
| */ | ||
| public function checkVersion() |
There was a problem hiding this comment.
We can use method injection here checkVersion(Release $release) instead :)
There was a problem hiding this comment.
I'll update to use this.
There was a problem hiding this comment.
Or, just app(Release::class)->.... That would be just fine too. I think we do that in StyleCI now eveywhere rather than method injection.
There was a problem hiding this comment.
Why we where not injecting? I forgot lol
There was a problem hiding this comment.
Because there's less code to write by not being parsed to inject. ;P
There was a problem hiding this comment.
Adding phpdoc is a ball ache, lol.
There was a problem hiding this comment.
Also, for performance reasons, we can resolve as late as possible, so we might bail out before we need to resolve an object.
There was a problem hiding this comment.
I was only complaining about phpdoc like 2 days a go ;)
|
Thanks Joe! |
| /** | ||
| * Cache instance. | ||
| * | ||
| * @var \Illuminate\Cache\CacheManager |
There was a problem hiding this comment.
Don't inject a cache manager. Inject the cache repository contract.
|
We good to merge this then? :) |
No description provided.