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

Fix session deprecation #380

Merged
merged 1 commit into from
Apr 15, 2022
Merged

Fix session deprecation #380

merged 1 commit into from
Apr 15, 2022

Conversation

dannyvw
Copy link
Contributor

@dannyvw dannyvw commented Mar 13, 2022

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? yes
Related tickets partially #335
License MIT

@dannyvw dannyvw requested a review from a team as a code owner March 13, 2022 14:53
use Symfony\Component\HttpFoundation\RequestStack;

/**
* TODO Remove after bumping to > Symfony 5.x
Copy link
Member

Choose a reason for hiding this comment

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

What about bumping minimal requirements to 4.4 already?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you meant 5.4, then I agree

@lchrusciel lchrusciel changed the base branch from master to 1.10 April 15, 2022 10:54
@lchrusciel lchrusciel added Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). Maintenance Configurations, READMEs, releases, etc. labels Apr 15, 2022
@lchrusciel lchrusciel merged commit 9192ee9 into Sylius:1.10 Apr 15, 2022
@lchrusciel
Copy link
Member

Thank you, Danny! 🎉

@@ -89,8 +103,14 @@ private function addFlash(string $type, string $message, array $parameters = [])
$message = $this->prepareMessage($message, $parameters);
}

if ($this->requestStack instanceof SessionInterface) {
Copy link
Member

Choose a reason for hiding this comment

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

Having afterthought, perhaps we should wrap it into our custom class, that would do this logic?

@dannyvw dannyvw deleted the session branch April 15, 2022 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). Maintenance Configurations, READMEs, releases, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants