-
Notifications
You must be signed in to change notification settings - Fork 1
Upgrade to Symfony 7.4 #136
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
base: main
Are you sure you want to change the base?
Conversation
- Upgrade PHPstan & fix issues - Add rector (no rules applied yet) - Fix phplint cache dir
| "surfnet/stepup-gssp-bundle": "^5.0", | ||
| "symfony/asset": "^6.4", | ||
| "symfony/console": "^6.4", | ||
| "surfnet/stepup-gssp-bundle": "dev-feature/update#e667625", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge upstream before merging
- Cleanup composer.json by removing non-existant namespaces - Apply rectors: * ThisCallOnStaticMethodToStaticCallRector * NullToStrictStringFuncCallArgRector * RemoveUselessReturnTagRector * AnnotationToAttributeRector * RenameAttributeRector - User Deprecated: Since symfony/http-kernel 6.3: Parameter "container.dumper.inline_class_loader" is deprecated, use ".container.dumper.inline_class_loader" instead.
sass-migrator division assets/scss/**/*.scss Remove warnings like during compilation: Module Warning (from ./node_modules/sass-loader/dist/cjs.js): Deprecation Using / for division outside of calc() is deprecated and will be removed in Dart Sass 2.0.0. Recommendation: math.div(18em, 14) or calc(18em / 14)
`js-yaml` was introduced as a resolution, then kept around. It's not needed.
8603c4c to
209c8e3
Compare
ci/qa/phpcpd
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given this project has NPM packages, you could opt for a jscpd alternative. As we have in Stepup-RA.
No deal breaker, but it could be nice to retain the CPD capability if possible.
See: https://github.com/OpenConext/Stepup-RA/blob/879c40a355078c1f88a2db15284d9e7962100a2f/ci/qa/jscpd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While testing the changes in my dev environment, I found out quite some SASS parser deprecation warnings remain in the project. No blocker for me, just saying..
Also: while testing, I noticed your frontend-install modifications you made in the other projects is not included here. Leaving me with permission issues after running the frontend install task. Maybe add the following section here?
"frontend-install": [
"yarn install",
"yarn encore prod",
"./bin/console cache:clear",
"@fix-dev-permissions"
],
"fix-dev-permissions": [
"chown -R www-data:www-data /var/www/html/var/log",
"chown -R www-data:www-data /var/www/html/var/cache"
],
I'm happy to report that your changes result in a functioning demo gssp that can be used for registrations and authentications. So big up there 👍
| "frontend-install": [ | ||
| "yarn install", | ||
| "yarn encore prod" | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe extend your frontend-install task with the suggestion I stated above?
No description provided.