Skip to content

PhpDumper heritable #10266

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

Closed
wants to merge 1 commit into from
Closed

PhpDumper heritable #10266

wants to merge 1 commit into from

Conversation

felixcarmona
Copy link

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

The PhpDumper is a really useful component, because is the responsible of write the project container cache, the people should be able to customize his framework.
For example in our personal case, we want to write the behavior of the getDefaultParameters() in the project container cache. (We need don't compile the parameters, and we want to rewrite this for:

 /**
  * @param $parameters
  * @return string
  */
 protected function getCodeForGetDefaultParameters($parameters)
 {
     return <<<EOF

  /**
   * Gets the default parameters.
   *
   * @return array An array of the default parameters
   */
  protected function getDefaultParameters()
  {
      return include("foo/our_custom_and_not_cachable_parameters.php");
  }

 EOF;

     return $code;
  }

Developers should be able to rewrite any behavior of this very important component of symfony.
Symfony should be a flexible framework.

@Tobion
Copy link
Contributor

Tobion commented Feb 28, 2014

👎 Because too internal and it would make the way the container is dumped/represented part of the API. Because overriding one method might require knowledge of the PHP code that is dumped by another one (like var names).

@fabpot
Copy link
Member

fabpot commented Feb 28, 2014

I do agree with @Tobion here and as a matter of fact, what you suggest would break in 2.5 if #10241 is merged.

@fabpot fabpot closed this Feb 28, 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.

5 participants