Skip to content

Conversation

davidromani
Copy link
Contributor

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

@fabpot
Copy link
Member

fabpot commented Nov 30, 2014

Thank you @davidromani.

@fabpot fabpot merged commit 21c32b5 into symfony:2.7 Nov 30, 2014
fabpot added a commit that referenced this pull request Nov 30, 2014
…ementMetadata (davidromani)

This PR was merged into the 2.7 branch.

Discussion
----------

[Validator] [Deprecated] Add a deprecation note about ElementMetadata

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

Commits
-------

21c32b5 add trigger message
@stof
Copy link
Member

stof commented Dec 4, 2014

The issue when putting this before the class declaration is that the notice is triggered only for the place triggering the autoload. So it will report a deprecation only in 1 place, not in all places using it

@nicolas-grekas
Copy link
Member

Which is good, because we do not want people to get flooded, but we only want to raise their awareness.
Then they can grep.

@stof
Copy link
Member

stof commented Dec 4, 2014

@nicolas-grekas the log message includes the location of the deprecated usage. Here, we are making things harder

@nicolas-grekas
Copy link
Member

I don't see how this is different with putting the notice inside the method/constructor?
Maybe be should adjust the logger for deprecation notices so that the n-1 frame in the stack is reported?

@stof
Copy link
Member

stof commented Dec 4, 2014

@nicolas-grekas in the constructor, it is reported for all places instantiating it. With the deprecation near the declaration, it is triggered when the autoloader includes the file, which gives you only the first usage.
Grepping to find similar usages might be complex because of dynamic access

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants