8000 [Validator] Fix TraceableValidator is reset on data collector instantiation by ogizanagi · Pull Request #24770 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Validator] Fix TraceableValidator is reset on data collector instantiation #24770

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

Jump to bottom
Merged
merged 1 commit into from
Oct 31, 2017
Merged

[Validator] Fix TraceableValidator is reset on data collector instantiation #24770

merged 1 commit into from
Oct 31, 2017

Conversation

ogizanagi
Copy link
Contributor
@ogizanagi ogizanagi commented Oct 31, 2017 •
edited
Loading
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

Calling reset from the constructor is wrong in this case as it'll reset the TraceableValidator, which means you'll never get collected data on the first request as the collector is instantiated after (on kernel response).

Another option would be to tag the debug.validator service with kernel.reset and remove the reset call from the collector.

@fabpot
Copy link
Member
fabpot commented Oct 31, 2017

Tagging with kernel.reset looks better to me as in "semantically" better.

@ogizanagi
Copy link
Contributor Author

Updated

@fabpot
Copy link
Member
fabpot commented Oct 31, 2017

Thank you @ogizanagi.

@fabpot fabpot merged commit f5855fb into symfony:3.4 Oct 31, 2017
fabpot added a commit that referenced this pull request Oct 31, 2017
@fabpot
bug #24770 [Validator] Fix TraceableValidator is reset on data collec…
782dc94 
…tor instantiation (ogizanagi)

This PR was merged into the 3.4 branch.

Discussion
----------

[Validator] Fix TraceableValidator is reset on data collector instantiation

| Q             | A
| ------------- | ---
| Branch?       | 3.4 <!-- see comment below -->
| Bug fix?      | yes
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets | N/A <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

Calling reset from the constructor is wrong in this case as it'll reset the `TraceableValidator`, which means you'll never get collected data on the first request as the collector is instantiated after (on kernel response).

Another option would be to tag the `debug.validator` service with `kernel.reset` and remove the reset call from the collector.

Commits
-------

f5855fb [Validator] Fix TraceableValidator is reset on data collector instantiation
@ogizanagi ogizanagi deleted the fix/3.4/validator_collector_reset branch October 31, 2017 17:51
This was referenced Nov 5, 2017
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

Assignees
No one assigned
Labels
Bug Status: Reviewed Validator
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ogizanagi @fabpot @carsonbot
0