Skip to content

Conversation

@Tobion
Copy link
Contributor

@Tobion Tobion commented Jul 1, 2019

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #32179
License MIT
Doc PR symfony/symfony-docs#...

Finish leftovers from #32282

* {@inheritdoc}
*/
protected function doSave(array $values, ?int $lifetime, array $addTagData = [], array $removeTagData = []): array
protected function doSave(array $values, int $lifetime, array $addTagData = [], array $removeTagData = []): array
Copy link
Contributor Author

@Tobion Tobion Jul 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parent has an int $lifetime and it does not seem to ever be called with null as lifetime. Also current implementation would be broken with null because two lines below it calls $this->doSaveCache($values, $lifetime); which requires an int as lifetime.

@Tobion Tobion merged commit 1919c7d into symfony:master Jul 2, 2019
Tobion added a commit that referenced this pull request Jul 2, 2019
This PR was merged into the 5.0-dev branch.

Discussion
----------

[Cache] finish type-hints

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #32179
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

Finish leftovers from #32282

Commits
-------

1919c7d [Cache] finish type-hints
@Tobion Tobion deleted the cache-finish-types branch July 2, 2019 14:52
@derrabus derrabus mentioned this pull request Jul 5, 2019
57 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants