file cache ttl #466
file cache ttl #466
Conversation
sveneld
force-pushed
the
feature-ttl
branch
3 times, most recently
from
1b5cf68
to
7dbd885
Compare
Codecov Report
@@ Coverage Diff @@
## master #466 +/- ##
============================================
+ Coverage 86.51% 86.51% +<.01%
Complexity 3158 3158
============================================
Files 204 204
Lines 8350 8353 +3
============================================
+ Hits 7224 7227 +3
Misses 1126 1126
Continue to review full report at Codecov.
|
| @@ -83,7 +83,8 @@ public function getConfigTreeBuilder() | |||
| ->addDefaultsIfNotSet() | |||
| ->children() | |||
| ->enumNode('driver')->defaultValue($defaultCacheDriver)->values(array('file', 'memory'))->end() | |||
| ->scalarNode('location')->defaultValue($home . '/.pdepend')->end() | |||
| ->scalarNode('location')->info('This value is only used for the file cache.')->defaultValue($home . '/.pdepend')->end() | |||
| ->integerNode('ttl')->info('This value is only used for the file cache. Value in seconds.')->defaultValue(30 * 24 * 60 *60)->end() | |||
| */ | ||
| public function __construct($cacheDir, $maxDays = 30) | ||
| public function __construct($cacheDir, $ttl) |
There was a problem hiding this comment.
Default value (30 * 86400) should be kept for backward compatibility as this is a public method. This means we should have a constant for 30 days in seconds so we don't duplicate it.
| { | ||
| $this->cacheDir = $cacheDir; | ||
| $this->minTime = time() - ($maxDays * 86400); | ||
| $this->ttl = time() - $ttl; |
There was a problem hiding this comment.
$ttl is a time-to-live (duration) but $this->ttl is actually an expiration timestamp (moment) so if we rename it, it should be more likely expirationTimestamp but not ttl.
| * @param string $cacheKey Unique key for this cache instance. | ||
| */ | ||
| public function __construct($root, $cacheKey = null) | ||
| public function __construct($root, $ttl, $cacheKey = null) |
There was a problem hiding this comment.
Here we should also have a default value. The easiest way is to put it everywhere $ttl is added.
| @@ -72,6 +79,7 @@ protected function setUp() | |||
| parent::setUp(); | |||
|
|
|||
| $this->cacheDir = $this->createRunResourceURI('cache') . '/'; | |||
| $this->cacheTtl = 24 * 60 * 60 * 30; | |||
There was a problem hiding this comment.
I would rather have ordered numbers from biggest unit to the smallest one.
| @@ -72,16 +79,17 @@ protected function setUp() | |||
| parent::setUp(); | |||
|
|
|||
| $this->cacheDir = $this->createRunResourceURI('cache'); | |||
| $this->cacheTtl = 24*60*60*30; | |||
There was a problem hiding this comment.
Add spaces (it could also use a common constant).
i made changes |
mayby you can add this changes to 2.7.1? |
|
No I can't, 2.7.1 is already released: https://github.com/pdepend/pdepend/releases/tag/2.7.1 |
|
when 2.7.2 tag will be released? |
|
If it last too many weeks to handle other issues planned for 2.7.2, we will release an intermediate version and postpone remaining tasks to 2.7.3. But meanwhile you can: To test the master branch when this PR will receive a second approval and be merged. |
Allow to configure file cache ttl in configuration file.
Type: feature
Issue: Fix #465
Breaking change: no