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

[Security] Declare exceptions that are already thrown by implementations #29779

Merged
merged 1 commit into from Jan 31, 2019
Merged

[Security] Declare exceptions that are already thrown by implementations #29779

merged 1 commit into from Jan 31, 2019

Conversation

umulmrum
Copy link
Contributor

@umulmrum umulmrum commented Jan 4, 2019

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

Adding exception declarations for PasswordEncoderInterface. I think it's a matter of opinion whether this change is a BC break. The BC promise doesn't cover such a case; I'd see it as a BC break to add exceptions in general, but in this case it's more of a "documentation" issue, as most implementations of the interface have been throwing those exceptions for years.

@umulmrum umulmrum changed the title Declare exceptions that are already thrown by implementations [Security] Declare exceptions that are already thrown by implementations Jan 4, 2019
@chalasr chalasr added this to the 3.4 milestone Jan 8, 2019
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know these annotations are really useful, but I've no strong objections either.

@chalasr
Copy link
Member

chalasr commented Jan 31, 2019

Thank you @umulmrum.

@chalasr chalasr merged commit f4cc30b into symfony:3.4 Jan 31, 2019
chalasr pushed a commit that referenced this pull request Jan 31, 2019
… implementations (umulmrum)

This PR was merged into the 3.4 branch.

Discussion
----------

[Security] Declare exceptions that are already thrown by implementations

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

Adding exception declarations for PasswordEncoderInterface. I think it's a matter of opinion whether this change is a BC break. The BC promise doesn't cover such a case; I'd see it as a BC break to add exceptions in general, but in this case it's more of a "documentation" issue, as most implementations of the interface have been throwing those exceptions for years.

Commits
-------

f4cc30b Declare exceptions that are already thrown by implementations
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

4 participants