Skip to content
This repository has been archived by the owner on Dec 28, 2020. It is now read-only.

Define all Cookie arguments explicitly #58

Merged
merged 4 commits into from May 6, 2019

Conversation

iambrosi
Copy link
Contributor

This adds support for Symfony 4.2, which deprecates not defining all the arguments as some of their default values are scheduled to change in Symfony 5.0.

This adds support for Symfony 4.2, which deprecates not defining all the arguments as some of their default values are scheduled to change in Symfony 5.0.
@iambrosi
Copy link
Contributor Author

iambrosi commented Feb 5, 2019

Any news on this?

@iambrosi
Copy link
Contributor Author

iambrosi commented Feb 5, 2019

@dunglas Could you please take a look at this?

This is the default value as of Symfony 4.2, and makes cookies safer as they won't be sent along with cross-site requests.

Also, added missing prediction when registering the cookie in the ResponseHeaderBag.
@iambrosi
Copy link
Contributor Author

iambrosi commented Apr 3, 2019

@dunglas Is this ready to be merged?

@dunglas
Copy link
Owner

dunglas commented Apr 4, 2019

Can you check why AppVeyor is red?

@iambrosi
Copy link
Contributor Author

iambrosi commented Apr 4, 2019

Travis is not red. AppVeyor is. It cannot find the path to c:\tools\php72.

@dunglas
Copy link
Owner

dunglas commented Apr 4, 2019

Can you try to replace php72 by php73 please? It should fix the build.

@sparksterz
Copy link

@iambrosi you can probably revert that last commit and just change the cinst -y php line to cinst -y php --version 7.2.17

Looks like even in your earlier builds it was pulling 7.3 and then failing, and when upgrading a dependency is looking for PHP < 7.3

This ensures appveyor uses PHP 7.2 and not a default version(currently 7.3)
@iambrosi
Copy link
Contributor Author

@dunglas Tests in AppVeyor have been fixed. Thanks @sparksterz for the tip! Also, fixed a new spec failure by a new method call added in symfony/symfony#29944

@matyax
Copy link

matyax commented May 6, 2019

Bump @dunglas. Is there any blocker to merging this PR?

@dunglas dunglas merged commit a329aa2 into dunglas:master May 6, 2019
@dunglas
Copy link
Owner

dunglas commented May 6, 2019

Thanks @iambrosi

@iambrosi iambrosi deleted the symfony-42-support branch May 6, 2019 16:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants