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

Php7.2 #257

Merged
merged 3 commits into from Dec 2, 2018
Merged

Php7.2 #257

merged 3 commits into from Dec 2, 2018

Conversation

greg0ire
Copy link
Contributor

@greg0ire greg0ire commented Dec 1, 2018

I am targeting this branch, because this PR has BC-breaks.

We cannot add the types to this signature without getting an error about
imcompatibility with \php_user_filter::filter:

> Declaration of Sonata\Exporter\Writer\CsvWriterTerminate::filter($in,
$out, int &$consumed, bool $closing): int should be compatible with
php_user_filter::filter($in, $out, &$consumed, $closing)
@greg0ire greg0ire added the major label Dec 1, 2018
@greg0ire
Copy link
Contributor Author

greg0ire commented Dec 1, 2018

Anticipates sonata-project/dev-kit#448

@greg0ire greg0ire requested a review from a team December 1, 2018 23:20
* @param resource $in
* @param resource $out
* @param int $consumed
* @param bool $closing
*/
public function filter($in, $out, &$consumed, $closing): int
Copy link
Contributor

Choose a reason for hiding this comment

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

why these arent types in the signature?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See commit message

@kunicmarko20 kunicmarko20 merged commit 3309705 into sonata-project:master Dec 2, 2018
@kunicmarko20
Copy link
Contributor

Thanks @greg0ire

@greg0ire greg0ire deleted the php7.2 branch December 2, 2018 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants