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

Resolve SF deprecation warnings #290

Merged
merged 4 commits into from
Aug 14, 2023
Merged

Conversation

MKodde
Copy link
Member

@MKodde MKodde commented Aug 9, 2023

Quite some warnings where resolved. The Behat output (see QA run on GHA) is now almost cleaned up. The one remaining warning there is the one triggered via the JS Translation Bundle:

User Deprecated: The Symfony\Bundle\TwigBundle\Loader\FilesystemLoader class is deprecated since version 4.3 and will be removed in 5.0; use Twig notation for templates instead.

We could rid ourself of that by simply upgrading the bundle, but that requires us to also upgrade to PHP8. So let's skip that for now.

@MKodde MKodde added the 4.2 label Aug 9, 2023
@MKodde MKodde force-pushed the maintenance/resolve-deprecations branch 2 times, most recently from d31202d to 0f7ab0d Compare August 9, 2023 13:41
Copy link
Contributor

@johanib johanib left a comment

Choose a reason for hiding this comment

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

few remarks, but no functional issues afaics

@@ -1,7 +1,7 @@
filter:
excluded_paths:
- "*/Tests/*"
- "app/DoctrineMigrations/*"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this not included in the previous line? Or is it case sensitive?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I was mislead by the removed line. I later found out that the newly added line was not having the effect I looked for. And I'm going to remove it once again. But your feedback is interesting, I do not think the excluded path regex is not performed in a case insensitive manner.

@@ -19,8 +19,8 @@
namespace Surfnet\StepupGateway\GatewayBundle\Saml\Proxy;

use Surfnet\StepupGateway\GatewayBundle\Saml\Exception\RuntimeException;
use Symfony\Component\HttpFoundation\Session\SessionBagInterface;
Copy link
Contributor

Choose a reason for hiding this comment

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

SessionBagInterface is not used in this file? Why is an import needed?

If this is really an error, consider tooling that removes unused imports.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, removed that import

The NamespacedAttributeBag is replaced with the AttributeBag
@MKodde MKodde force-pushed the maintenance/resolve-deprecations branch from 0f7ab0d to 4f64c28 Compare August 14, 2023 08:27
Migrations are managed via Stepup-Middleware. Having them in this
project makes no sense. And might cause confusion
@MKodde MKodde force-pushed the maintenance/resolve-deprecations branch from 4f64c28 to f0c1dc7 Compare August 14, 2023 08:35
@MKodde MKodde merged commit 44eb9fa into develop Aug 14, 2023
2 checks passed
@MKodde MKodde deleted the maintenance/resolve-deprecations branch August 14, 2023 08:46
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.

2 participants