Skip to content

Use a better exception when using make:auth with a form #474

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 31, 2020
Merged

Use a better exception when using make:auth with a form #474

merged 1 commit into from
Jan 31, 2020

Conversation

@lyrixx
Copy link
Member

@lyrixx lyrixx commented Sep 24, 2019

No description provided.

@romaricdrigon
Copy link
Contributor

romaricdrigon commented Oct 12, 2019

Hello,

I have some mixed feelings about using a BadMethodCallException.
From PHP manual, Exception thrown if a callback refers to an undefined method or if some arguments are missing., I feel like we don't quite have the same semantic.
Is there any reason why this exception is better?
Personally, I would prefer a LogicException.

@romaricdrigon romaricdrigon added the Status: Waiting Feedback Needs feedback from the author label Oct 12, 2019
@lyrixx
Copy link
Member Author

lyrixx commented Oct 14, 2019

Anyway, just exception is not cool

@romaricdrigon
Copy link
Contributor

romaricdrigon commented Oct 16, 2019

Sure. Then could you please modify with LogicException, and fix failing test?
Thank you

@romaricdrigon romaricdrigon added Status: Needs Work Additional work is needed and removed Status: Waiting Feedback Needs feedback from the author labels Oct 16, 2019
@lyrixx lyrixx force-pushed the badmethodcall branch 2 times, most recently from e1af954 to c8ee4c5 Compare November 26, 2019 18:42
@lyrixx
Copy link
Member Author

lyrixx commented Nov 26, 2019

I fixed it, but it seems that tests are broken

@lyrixx
Copy link
Member Author

lyrixx commented Jan 30, 2020

ping @romaricdrigon

@weaverryan
Copy link
Member

weaverryan commented Jan 31, 2020

Thanks Grégoire!

weaverryan added a commit that referenced this pull request Jan 31, 2020
@weaverryan
minor #474 Use a better exception when using make:auth with a form (l…
61cf560 
…yrixx)

This PR was merged into the 1.0-dev branch.

Discussion
----------

Use a better exception when using make:auth with a form

Commits
-------

ee1763b Use a better exception when using make:auth with a form
@weaverryan weaverryan merged commit ee1763b into symfony:master Jan 31, 2020
@jrushlow jrushlow mentioned this pull request Jan 31, 2020
Merged
weaverryan added a commit that referenced this pull request Jan 31, 2020
@weaverryan
minor #539 fix security logout fixture (jrushlow)
674aa82 
This PR was squashed before being merged into the 1.0-dev branch (closes #539).

Discussion
----------

fix security logout fixture

#474 changed implemented \LogicException instead of \Exception for make:auth. Fixture still produced \Exception for assertion.

Commits
-------

e5cb483 fix security logout fixture
@lyrixx lyrixx deleted the badmethodcall branch January 31, 2020 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@maxhelias maxhelias maxhelias approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Status: Needs Work Additional work is needed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lyrixx @romaricdrigon @weaverryan @maxhelias