10000 [Config] Delegate creation of ConfigCache instances to a factory. by mpdude · Pull Request #14178 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Config] Delegate creation of ConfigCache instances to a factory. #14178

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.

Sign up for GitHub

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

Closed
wants to merge 24 commits into from
Closed

[Config] Delegate creation of ConfigCache instances to a factory. #14178

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
02159ae
Delegate creation of ConfigCache instances to a factory.
mpdude Apr 2, 2015
98aa48b
Fix wrong method name
mpdude Apr 2, 2015
3744edf
Add E_USER_DEPRECATED warning
mpdude Apr 2, 2015
282a75a
CS fix
mpdude Apr 2, 2015
ae29b82
CS fixes
mpdude Apr 2, 2015
1730fc3
Remove "constructor" docblocks
mpdude Apr 2, 2015
770ee09
DocBlock fixes
mpdude Apr 2, 2015
2a058fd
Fix DocBlock
mpdude Apr 2, 2015
8491dfd
Fix deprecation warnings
mpdude Apr 2, 2015
beb307c
Fix unnecessary re-initialization from cache
mpdude Apr 2, 2015
dbec920
Add deprecation notice in UPGRADE-2.7
mpdude Apr 4, 2015
5a91716
Tests need a recent version of the Config component
mpdude Apr 4, 2015
f2c3a24
Explicitly add symfony/config dependency in an desperate attempt to m…
mpdude Apr 4, 2015
35488bd
Suggestions from GH
mpdude Apr 5, 2015
17a55ec
FabBot compliance
mpdude Apr 6, 2015
07ecf57
Bump composer dep
mpdude Apr 6, 2015
a89db7a
As in aa82fb065d970c1fd02bf35790e4da8970f45e7b
mpdude Apr 6, 2015
7938349
Update composer.json
mpdude Apr 6, 2015
cb9917f
Don't mess with composer.json where it is not necessary
mpdude Apr 7, 2015
2016537
The HttpKernel for sure depends on the Config component
mpdude Apr 7, 2015
beb0a92
Cleanup composer.json
mpdude Apr 7, 2015
afee2bb
Make sure at least symfony/config 2.7 is installed when loaded as a t…
mpdude Apr 7, 2015
4832e1a
symfony/config dev-master cannot be used unless changes from this PR …
mpdude Apr 7, 2015
7f90bae
Grammar fix in comment
mpdude Apr 7, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix wrong method name
  • Loading branch information
@mpdude
mpdude committed Apr 6, 2015
commit 98aa48b0dc1862c5a269039708838b40b19c8761
2 changes: 1 addition & 1 deletion src/Symfony/Component/Config/ConfigCache.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public function __construct($file, $debug)
* Gets the cache file path.
*
* @return string The cache file path
* @deprecated since 2.7, to be removed in 3.0. Use getFilePath() instead.
* @deprecated since 2.7, to be removed in 3.0. Use getPath() instead.
*/
public function __toString()
{
Copy link
Member

Choose a reason for hiding this comment

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

You need to add a deprecation notice (look for trigger_error() in Symfony 2.7 for some examples).

Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Config/ConfigCacheInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ interface ConfigCacheInterface
/**
* Gets the cache file path.
*
* @deprecated since version 2.7, to be removed in 3.0.
* @deprecated since 2.7, to be removed in 3.0. Use getPath() instead.
* @return string The cache file path
Copy link
Member

Choose a reason for hiding this comment

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

@deprecated should be moved below this one and an empty line should be added between the two directives

*/
public function __toString();
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be a better idea to either remove the __toString() from the interface or move it to a deprecated interface which can extend this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right! I'm going to remove it. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

I would remove it from the interface and update changelog & upgrade files to only mention the method in the ConfigCache class.

Expand All @@ -36,7 +36,7 @@ public function getPath();
/**
* Checks if the cache is still fresh.
*
* @return Boolean true if the cache is fresh, false otherwise
* @return bool true if the cache is fresh, false otherwise
Copy link
Member

Choose a reason for hiding this comment

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

no need for the extra spacing here

And I believe "whether the cache is fresh" is more consistent with other occurences in the Symfony code base

*/
public function isFresh();

Expand Down
0