Skip to content
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

Improve removedFunctionParameters sniff. #163

Merged

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Aug 13, 2016

  • Add code to handle parameters which are first deprecated and only later removed. While a parameter is deprecated, but not yet removed, the sniff will show a warning instead of an error.
  • Add deprecation version for the existing parameter checks.
  • Add some additional removal parameters.
  • Minor rewrite of the actual error message.

Includes unit tests and adjusted documentation.

Question: the sniff name does no longer really cover what this sniff does. Should the sniff be renamed ? and if so, to what ? DeprecatedFunctionParameters ?

* Add code to handle parameters which are first deprecated and only later removed. While a parameter is deprecated, but not yet removed, the sniff will show a warning instead of an error.
* Add deprecation version for the existing parameter checks.
* Add some additional removal parameters.
* Minor rewrite of the actual error message.

Includes unit tests and adjusted documentation.

Question: the sniff name does no longer really cover what this sniff does. Should the sniff be renamed ? and if so, to what ? `DeprecatedFunctionParameters` ?
@wimg
Copy link
Member

wimg commented Aug 13, 2016

Good question. I've been planning to overhaul the names and set up a naming convention. There are some tests and test files that are incorrectly or even inconsistently named too. Let's leave it the way it is for now and fix it all in 1 go.

@wimg wimg merged commit c511a43 into PHPCompatibility:master Aug 13, 2016
@jrfnl jrfnl deleted the feature/more-removed-function-params branch August 14, 2016 01:20
@jrfnl jrfnl added this to the 7.0.3 milestone Apr 30, 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.

None yet

2 participants