Skip to content
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

[FrameworkBundle] cache:clear command fills *.php.meta files with wrong data #12137

Closed
wants to merge 6 commits into from

Conversation

Strate
Copy link
Contributor

@Strate Strate commented Oct 5, 2014

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #12110
License MIT

Test and fix of ticket #12110

This unit test shows, that right after executing 'cache:clear' command (which warms up cache also),
*.php.meta files contains wrong information
use Symfony\Component\HttpKernel\Bundle\BundleInterface;
use Symfony\Component\HttpKernel\Kernel;

class AppKernel extends Kernel
Copy link
Contributor

Choose a reason for hiding this comment

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

TestAppKernel would sound better. Some IDE's (such as eclipse) will index this file and provide auto complete on it or quick searches. That means that besides of the AppKernel in your own applications, you will always see a second pop up. This can be confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am agree with you. But There is also exists at least Symfony\Bundle\FrameworkBundle\Tests\Functional\app\AppKernel class. So, I'll fix it, but later (probably with CacheClearCommand fix)

@Strate
Copy link
Contributor Author

Strate commented Oct 7, 2014

I don't know ahy tests are failed. Fail does not related to this PR.

@Strate
Copy link
Contributor Author

Strate commented Oct 9, 2014

Could someone review?

@@ -0,0 +1,2 @@
framework:
secret: test
Copy link
Member

Choose a reason for hiding this comment

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

you should add a newline at the end of the file

1. Fix code review marks
2. Added test for kernel file existence in meta file.
@Strate
Copy link
Contributor Author

Strate commented Oct 11, 2014

@xabbuh fixes, except wrapping assertion. I like wrapped :)
@stof I wrote test for your note.


use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\HttpKernel\Bundle\BundleInterface;
Copy link
Member

Choose a reason for hiding this comment

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

The BundleInterface is never used.

@Strate
Copy link
Contributor Author

Strate commented Oct 20, 2014

Test failures does not related to code. Could someone merge/reject this?

@stof
Copy link
Member

stof commented Nov 21, 2014

👍

@fabpot
Copy link
Member

fabpot commented Nov 21, 2014

Thank you @Strate.

fabpot added a commit that referenced this pull request Nov 21, 2014
…es with wrong data (Strate)

This PR was submitted for the 2.5 branch but it was merged into the 2.3 branch instead (closes #12137).

Discussion
----------

[FrameworkBundle] cache:clear command fills *.php.meta files with wrong data

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #12110
| License       | MIT

Test and fix of ticket #12110

Commits
-------

76273bf [FrameworkBundle] cache:clear command fills *.php.meta files with wrong data
@fabpot fabpot closed this Nov 21, 2014
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.

None yet

5 participants