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

preventing call to undefined method isSent #13821

Closed
wants to merge 5 commits into from
Closed

Conversation

Idrinth
Copy link
Contributor

@Idrinth Idrinth commented Feb 12, 2019

Hello!

  • Type: bug fix
  • Link to issue: -

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the Contributing Guidelines?
  • I have checked that another pull request for this purpose does not exist.
  • I wrote some tests for this PR.

Small description of change:

Currently isSent is only implemented by Response and not part of the ResponseInterface. Therefore calling it on instances of the interfaces is not a reasonable solution.
Adjusting the interface would break backwards compatibility, so adding the check seems like the more sensible option.

phalcon/mvc/micro.zep Outdated Show resolved Hide resolved
@Idrinth
Copy link
Contributor Author

Idrinth commented Feb 12, 2019

I'm not sure how CacheTest::testDataFileCacheIncrement Failed asserting that 7 matches expected null. could be related. Do you have any idea where I could dig for the connection between these changes and the failure, @Jeckerson ?

@CameronHall
Copy link
Contributor

@Idrinth Thank you for your contribution!

We are no longer accepting pull requests to 3.4.x. If this change is still applicable to the 4.0.x branch can you please target that.

@Idrinth
Copy link
Contributor Author

Idrinth commented Feb 18, 2019

still an issue in master and 4.0.x, but with 4 being breaking it might be a better choice to add the missing method to the interface, wouldn't it @CameronHall ?

@CameronHall
Copy link
Contributor

Absolutely @Idrinth!

@Idrinth
Copy link
Contributor Author

Idrinth commented Feb 18, 2019

replaced with #13836 to solve issue in 4.0.x

@Idrinth Idrinth closed this Feb 18, 2019
@niden niden added documentation Documentation required and removed documentation Documentation required labels Apr 9, 2019
@niden niden added the 4.0 label Dec 2, 2019
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