Skip to content

Conversation

@stof
Copy link
Member

@stof stof commented Aug 29, 2015

Injecting the Session in exceptions is now deprecated in favor of injecting the driver.

This is a backport of 761bb79 which was done as part of #542 to refactor the driver in 2.0.
Backporting it with a BC layer means we give people instantiating our exception themselves (me at work for instance) an easier upgrade path, because they can prepare things before Mink 2.0.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm wondering whether we should throw exceptions in place now and deprecate this method (this was added in 1.6 just so that our own child classes can avoid using the deprecated Session getter)

Copy link
Member

Choose a reason for hiding this comment

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

Isn't this just an internal method that contained some logic before, but not now? If so, then we'd better show exception where needed instead of calling method, that throws them for us. This would also give us shorter stacktrace for that exception.

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed. Precisely my point.
there was no logic before, but it allowed to access the session without using the deprecated getter

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@stof stof force-pushed the exception_dependency branch from c2729cd to 22adbb0 Compare August 29, 2015 09:28
Copy link
Member

Choose a reason for hiding this comment

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

What is the point of using error hiding (via @) if we want to trigger an error?

Copy link
Member Author

Choose a reason for hiding this comment

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

see #662

@stof stof force-pushed the exception_dependency branch 2 times, most recently from 00fa3d9 to c2ec8e6 Compare August 29, 2015 09:53
stof added 2 commits August 29, 2015 13:05
Injecting the Session in exceptions is now deprecated in favor of
injecting the driver.
@stof stof force-pushed the exception_dependency branch from c2ec8e6 to dbde834 Compare August 29, 2015 11:09
stof added a commit that referenced this pull request Aug 29, 2015
Remove the dependency on the Session in expectation exceptions
@stof stof merged commit cb36303 into minkphp:master Aug 29, 2015
@stof stof deleted the exception_dependency branch August 29, 2015 18:10
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.

2 participants