Skip to content

[Debug] Restoring back the state of the Debug component (1st step) #32377

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
Jul 9, 2019

Conversation

yceruto
Copy link
Member

@yceruto yceruto commented Jul 4, 2019

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

After a good discussion with @nicolas-grekas, we made the decision to split the current ErrorCatcher component into several:

  • ErrorHandler it would be the Debug component before these changes Add ErrorHandler component #31065, with everything related to ErrorHandler, Debug, DebugClassLoader classes and change its name.
  • ErrorDumper it would be the current ErrorCatcher but with FlattenException + the new error renderer system only.

This is the first step, then we can deprecate everything for the Debug component in favor of the ErrorHandler and ErrorDumper components, BUT without moving any code !!, that would give us more freedom to do it correctly in the new components.

NOTE: For this PR I've copy the Debug component directory from the revision prior to merged commit #31065 in 4.4 branch.

@fabpot
Copy link
Member

fabpot commented Jul 4, 2019

ErrorRendeder instead of ErrorDumper?

@yceruto
Copy link
Member Author

yceruto commented Jul 4, 2019

I will also remove from the current ErrorCatcher everything unrelated to the future ErrorDumper and make it work. Later, as last step, let's create/rename both components to ErrorHandler and ErrorDumper respectively.

Status: Needs Work

@yceruto
Copy link
Member Author

yceruto commented Jul 4, 2019

Status: Needs Review

(low/high Travis's failures are gone after merge)

@yceruto
Copy link
Member Author

yceruto commented Jul 4, 2019

ErrorRenderer instead of ErrorDumper?

That one is nice too. Personally, I like ErrorDumper it feels like VarDumper but for errors. Let's rename it for another PR.

@yceruto yceruto force-pushed the debug_component branch from 47ecec9 to b824694 Compare July 5, 2019 11:12
@yceruto
Copy link
Member Author

yceruto commented Jul 5, 2019

Thanks Nicolas! now Travis's low tests pass.

Status: Needs Review

@nicolas-grekas
Copy link
Member

It would make sense to me to remove error-catcher from the composer.json of the Debug component.
This means using the legacy FlattenException in ExceptionHandler, which is OK.

@yceruto yceruto force-pushed the debug_component branch 3 times, most recently from 6c7570c to fa47beb Compare July 8, 2019 13:12
@yceruto
Copy link
Member Author

yceruto commented Jul 8, 2019

@nicolas-grekas it's done now, thanks.

@nicolas-grekas
Copy link
Member

Thank you @yceruto.

@nicolas-grekas nicolas-grekas merged commit eda49e2 into symfony:4.4 Jul 9, 2019
nicolas-grekas added a commit that referenced this pull request Jul 9, 2019
…(1st step) (yceruto)

This PR was squashed before being merged into the 4.4 branch (closes #32377).

Discussion
----------

[Debug] Restoring back the state of the Debug component (1st step)

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

After a good discussion with @nicolas-grekas, we made the decision to split the current `ErrorCatcher` component into several:
 * `ErrorHandler` it would be the Debug component before these changes #31065, with everything related to ErrorHandler, Debug, DebugClassLoader classes and change its name.
 * `ErrorDumper` it would be the current ErrorCatcher but with FlattenException + the new error renderer system only.

This is the first step, then we can deprecate everything for the Debug component in favor of the ErrorHandler and ErrorDumper components, **BUT without moving any code !!**, that would give us more freedom to do it correctly in the new components.

NOTE: For this PR I've copy the `Debug` component directory from the revision prior to merged commit  #31065 in 4.4 branch.

Commits
-------

eda49e2 [Debug] Restoring back the state of the Debug component (1st step)
@yceruto yceruto deleted the debug_component branch July 9, 2019 10:49
nicolas-grekas added a commit that referenced this pull request Jul 11, 2019
…nly) (yceruto)

This PR was squashed before being merged into the 4.4 branch (closes #32470).

Discussion
----------

Rename ErrorCatcher to ErrorRenderer (rendering part only)

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

Follow up #32377

Commits
-------

fb5b042 Rename ErrorCatcher to ErrorRenderer (rendering part only)
symfony-splitter pushed a commit to symfony/web-profiler-bundle that referenced this pull request Jul 11, 2019
…nly) (yceruto)

This PR was squashed before being merged into the 4.4 branch (closes #32470).

Discussion
----------

Rename ErrorCatcher to ErrorRenderer (rendering part only)

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

Follow up symfony/symfony#32377

Commits
-------

fb5b0429b2 Rename ErrorCatcher to ErrorRenderer (rendering part only)
symfony-splitter pushed a commit to symfony/http-kernel that referenced this pull request Jul 11, 2019
…nly) (yceruto)

This PR was squashed before being merged into the 4.4 branch (closes #32470).

Discussion
----------

Rename ErrorCatcher to ErrorRenderer (rendering part only)

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

Follow up symfony/symfony#32377

Commits
-------

fb5b042 Rename ErrorCatcher to ErrorRenderer (rendering part only)
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request Jul 11, 2019
…nly) (yceruto)

This PR was squashed before being merged into the 4.4 branch (closes #32470).

Discussion
----------

Rename ErrorCatcher to ErrorRenderer (rendering part only)

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

Follow up symfony/symfony#32377

Commits
-------

fb5b0429b2 Rename ErrorCatcher to ErrorRenderer (rendering part only)
symfony-splitter pushed a commit to symfony/messenger that referenced this pull request Jul 11, 2019
…nly) (yceruto)

This PR was squashed before being merged into the 4.4 branch (closes #32470).

Discussion
----------

Rename ErrorCatcher to ErrorRenderer (rendering part only)

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

Follow up symfony/symfony#32377

Commits
-------

fb5b0429b2 Rename ErrorCatcher to ErrorRenderer (rendering part only)
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
symfony-splitter pushed a commit to symfony/messenger that referenced this pull request Jan 28, 2020
…nly) (yceruto)

This PR was squashed before being merged into the 4.4 branch (closes #32470).

Discussion
----------

Rename ErrorCatcher to ErrorRenderer (rendering part only)

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

Follow up symfony/symfony#32377

Commits
-------

fb5b0429b2 Rename ErrorCatcher to ErrorRenderer (rendering part only)
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