Skip to content

Conversation

@W0rma
Copy link
Contributor

@W0rma W0rma commented Mar 22, 2020

Copy link
Contributor

@alquerci alquerci left a comment

Choose a reason for hiding this comment

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

Thanks @W0rma to work on it.

👌 LGTM except that travis build are already a ready PR #216

Then this PR depends on #216


// GET parameters
$this->getParameters = get_magic_quotes_gpc() ? sfToolkit::stripslashesDeep($_GET) : $_GET;
if (version_compare(PHP_VERSION, '5.4', '<') && get_magic_quotes_gpc())
Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend to use 7.4.0-dev instead of 7.4.

Note that pre-release versions, such as 5.3.0-dev, are considered lower than their final release counterparts (like 5.3.0).

source https://www.php.net/manual/en/function.version-compare.php

Copy link
Contributor

Choose a reason for hiding this comment

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

@alquerci you mean 5.4.0-dev, right? :)

Copy link
Contributor

@alquerci alquerci Mar 23, 2020

Choose a reason for hiding this comment

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

Ha, yes.

I through that patch is related to PHP 7.4 support. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the hint. Adjusted to 5.4.0-dev

Copy link
Contributor

@alquerci alquerci left a comment

Choose a reason for hiding this comment

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

👍 except to remove travis patch as it is done on #216

Then I propose to make this PR dependent on #216
And add PHP 7.3 and 7.4 on the list.

@mentalstring
Copy link
Contributor

I don't make a point in having my PR in — this one does the same and beyond 👍— I'll close #216 .

Copy link
Contributor

@mentalstring mentalstring left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@j0k3r j0k3r merged commit 60275e4 into FriendsOfSymfony1:master Apr 2, 2020
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.

5 participants