-
-
Notifications
You must be signed in to change notification settings - Fork 34
removed dependency on nette/caching #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
1581a88
to
59edb76
Compare
Good idea, I'll review the implementation in the morning |
|
||
|
||
/** | ||
* @return string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be @return array
(or @return mixed
in theory, as it may be overridden to return anything that is serializable, but i like @return array
more.
/** @deprecated */ | ||
public function setCacheStorage(Nette\Caching\IStorage $storage) | ||
{ | ||
trigger_error(__METHOD__ . '() is deprecated; use setTempDirectory() instead.', E_USER_DEPRECATED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For FileStorage
instances, we may for compatibility reasons call setTempDirectory
with $storage->dir
(private, so minor hacking would be required).
For other storages, exception should be thrown as RobotLoader is entirely broken and will not work unless fixed by programmer.
* @internal | ||
*/ | ||
public function rebuildCallback() | ||
private function refreshCache() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return value of refreshCache
is never used
return; | ||
} | ||
$file = $this->getCacheFile(); | ||
$list = @include $file; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe assign directly to $this->classes
?
that would allow omitting $list = $this->classes
and $this->classes = $list
lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this could be…
} | ||
|
||
$file = $this->getCacheFile(); | ||
$code = '<?php return ' . var_export($this->classes, TRUE) . ';'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about better formatting (newlines after <?php
, replacing path prefix with __DIR__
, ...) but it's probably not worth it – it used to be totally unreadable serialized format, even default var_export
formatting is a lot more readable.
Looks like updated. Could you review @JanTvrdik, please? |
} | ||
|
||
|
||
/** | ||
* @return array | ||
*/ | ||
protected function getKey() | ||
private function getCacheFile() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@return string
fe1fde2
to
7030c2f
Compare
@JanTvrdik thx for review |
Cache file is ordinary PHP file.