Skip to content

[FrameworkBundle] drop hard dependency on the Stopwatch component #23148

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

Merged
merged 1 commit into from
Jun 12, 2017

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Jun 12, 2017

Q A
Branch? 3.4
Bug fix? no
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets related to symfony/flex#14
License MIT
Doc PR

@stof
Copy link
Member

stof commented Jun 12, 2017

does TwigBundle works fine for the Stopwatch extension if the service is not always available anymore ?

fabpot added a commit to symfony/profiler-pack that referenced this pull request Jun 12, 2017
This PR was merged into the master branch.

Discussion
----------

add dependency on the Stopwatch component

This is the counterpart of symfony/symfony#23148. Using the profiler without the Stopwatch component makes it lack a lot of useful information. However, in production you will probably not need it all. Thus, I suggest to require the component in the profiler pack to allow the FrameworkBundle to drop the dependency and thus decreasing the required deps for the production environment.

Commits
-------

06d5b5b add dependency on the Stopwatch component
@xabbuh
Copy link
Member Author

xabbuh commented Jun 12, 2017

Good question, I did not check what's happing if the debug.stopwatch service is not present as it would already be ignored then. So if that was not working properly, it would already have to be fixed in older branches IMO.

@stof
Copy link
Member

stof commented Jun 12, 2017

@xabbuh well, currently, the debug.stopwatch service is always present. So we clearly need to check the impact of this change

@fabpot
Copy link
Member

fabpot commented Jun 12, 2017

@stof all usage in Symfony uses on-invalid=null, so we're good here.

Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

As the stopwatch service is only available in the dev env, this looks good to me. We cannot break production inadvertently.

@stof
Copy link
Member

stof commented Jun 12, 2017

As the stopwatch service is only available in the dev env, this looks good to me. We cannot break production inadvertently.

@fabpot that's not true. This service was defined in debug_prod.xml, and so loaded in all environments

@stof
Copy link
Member

stof commented Jun 12, 2017

However, it won't change anything for fullstack users

@fabpot
Copy link
Member

fabpot commented Jun 12, 2017

@stof indeed, I thought I remembered it being defined in debug.xml Anyway, you're right about not being a problem for people using the full-stack. That's why I was also able to remove many other deps in 3.2.

@fabpot
Copy link
Member

fabpot commented Jun 12, 2017

Thank you @xabbuh.

@fabpot fabpot merged commit 8d70ca0 into symfony:3.4 Jun 12, 2017
fabpot added a commit that referenced this pull request Jun 12, 2017
…h component (xabbuh)

This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle] drop hard dependency on the Stopwatch component

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | related to symfony/flex#14
| License       | MIT
| Doc PR        |

Commits
-------

8d70ca0 drop hard dependency on the Stopwatch component
@xabbuh xabbuh deleted the drop-stopwatch-requirement branch June 12, 2017 18:50
@xabbuh
Copy link
Member Author

xabbuh commented Jun 13, 2017

FYI, I have checked the code of the classes where null could be injected instead of the debug.stopwatch service and all of them properly deal with null being passed.

fabpot added a commit that referenced this pull request Jun 13, 2017
…pwatch (stof)

This PR was merged into the 3.4 branch.

Discussion
----------

Fix the usage of FrameworkBundle in debug mode without Stopwatch

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

#23148 removed the loading of the debug.xml file when Stopwatch is not installed.
While all services defined in it are related to Stopwatch, the parameter was not (it is related to dumping the debug container in the compiler pass), which was breaking the usage of the bundle in debug mode without Stopwatch (exception triggered in the compiler pass)

Commits
-------

2ea26c1 Fix the usage of FrameworkBundle in debug mode without Stopwatch
This was referenced Oct 18, 2017
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