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

Symfony 5 upgrade #565

Merged
merged 50 commits into from
Jan 24, 2023
Merged

Symfony 5 upgrade #565

merged 50 commits into from
Jan 24, 2023

Conversation

Shaky212
Copy link
Contributor

@Shaky212 Shaky212 commented Nov 22, 2022

A mighty PR where multiple disciplines entailing the Symfony upgrade come together.

Among them:
#562 Upgrade to Symfony 4 (in preparation to this SF5 upgrade)
#571 Move to Github actions for test integrations

At first these tasks where created separate. But to keep things more tight, I chose to merge/rebase them into this branch.

@Shaky212 Shaky212 changed the title Symfony 5 upgrade continued Symfony 5 upgrade Nov 22, 2022
@Shaky212 Shaky212 force-pushed the feature/symfony-5-upgrade-continued branch from 42b6639 to 84023f9 Compare November 24, 2022 09:43
@Shaky212 Shaky212 force-pushed the feature/symfony-5-upgrade branch 11 times, most recently from 3a2b53f to 859978a Compare December 8, 2022 15:57
@Shaky212 Shaky212 force-pushed the feature/symfony-5-upgrade-continued branch from 84023f9 to 53845dc Compare December 9, 2022 11:15
@MKodde MKodde force-pushed the feature/symfony-5-upgrade-continued branch from 53845dc to a152117 Compare January 4, 2023 07:27
@MKodde MKodde mentioned this pull request Jan 4, 2023
@MKodde MKodde force-pushed the feature/symfony-5-upgrade-continued branch from 5abd989 to 03f7d96 Compare January 5, 2023 07:38
@MKodde MKodde changed the base branch from feature/symfony-5-upgrade to develop January 5, 2023 07:41
@MKodde MKodde force-pushed the feature/symfony-5-upgrade-continued branch 5 times, most recently from a4adf2c to 9c7ab01 Compare January 5, 2023 13:17
@quartje quartje force-pushed the feature/symfony-5-upgrade-continued branch 2 times, most recently from 8f62fc1 to 4e01151 Compare January 11, 2023 07:47
@Shaky212 Shaky212 requested a review from MKodde January 17, 2023 16:04
Copy link
Contributor

@MKodde MKodde left a comment

Choose a reason for hiding this comment

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

What an effort! Hats off to you @Shaky212

See some feedback that you might need to do something with, but at the other hand. Might also want to fix in a successive PR. You be the judge.

For now, we should first get a 4.4 release before merging this to develop.

.env~Stashed changes Outdated Show resolved Hide resolved
build.xml Outdated Show resolved Hide resolved
docker/conf/000-default-dev.conf Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
ansible/roles/spdashboard/tasks/install-branch.yml Outdated Show resolved Hide resolved
public/index.php Outdated Show resolved Hide resolved
@Shaky212 Shaky212 force-pushed the feature/symfony-5-upgrade-continued branch from 4d5a356 to 63891df Compare January 23, 2023 10:16
Copy link
Contributor

@MKodde MKodde left a comment

Choose a reason for hiding this comment

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

I only reviewed the newly added commits and found some merge issues in the composer.json.

composer.json Outdated
"assets:install %PUBLIC_DIR%": "symfony-cmd"
},
"symfony-scripts": [
"Incenteev\\ParameterHandler\\ScriptHandler::buildParameters",
Copy link
Contributor

Choose a reason for hiding this comment

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

This Symfony script will not work anymore as we removed the parameters handler dependency.

composer.json Outdated
Comment on lines 87 to 89
"Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::clearCache",
"Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::installAssets",
"Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::installRequirementsFile"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same for the DistributionBundle scripts. Output of composer install

Class Incenteev\ParameterHandler\ScriptHandler is not autoloadable, can not call symfony-scripts script
Class Sensio\Bundle\DistributionBundle\Composer\ScriptHandler is not autoloadable, can not call symfony-scripts script
Class Sensio\Bundle\DistributionBundle\Composer\ScriptHandler is not autoloadable, can not call symfony-scripts script
Class Sensio\Bundle\DistributionBundle\Composer\ScriptHandler is not autoloadable, can not call symfony-scripts script

composer.json Outdated
Comment on lines 152 to 158
"symfony-app-dir": "app",
"symfony-bin-dir": "bin",
"symfony-var-dir": "var",
"symfony-web-dir": "web",
"symfony-tests-dir": "tests",
"symfony-assets-install": "relative",
"branch-alias": null
Copy link
Contributor

Choose a reason for hiding this comment

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

This is SF3 stuff..

Upgrade Symfony 4

Upgrade Symfony 4
Shaky212 and others added 22 commits January 23, 2023 14:10
Different situations are covered when creating and
changing entities. These include creating an entity,
whether or not based on a copy, or editing an existing
entity. In addition, the environment is decisive:
test, production or otherwise
In case of a new (copy) entity, a connection request must
also be submitted. These situations and error handling
are now a bit more streamlined.
The SAML bundle is now capable of running and evaluating SAML authentication
requests (and SAML responses). This commit removes the old logic that
was hard coded into the SP Dashboard, and integrates the new
authentication system from the Stepup saml bundle.
The previous lifetime guard solution was dropped in favor of using the
SF built in session lifetime features.
This is now (as of SF5) managed using .env vars that determine the env +
debug mode from the .env vars.
Some of the code using the code needed some changes as some of the
methods where changed slightly
@Shaky212 Shaky212 force-pushed the feature/symfony-5-upgrade-continued branch from 9975792 to c0acecd Compare January 23, 2023 13:10
@Shaky212 Shaky212 force-pushed the feature/symfony-5-upgrade-continued branch from 79deccc to e799cda Compare January 23, 2023 15:54
@Shaky212 Shaky212 requested a review from MKodde January 24, 2023 08:05
Copy link
Contributor

@MKodde MKodde left a comment

Choose a reason for hiding this comment

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

What a pickle, thanks for persevering on this one!

@Shaky212 Shaky212 merged commit d44b7e0 into develop Jan 24, 2023
@Shaky212 Shaky212 deleted the feature/symfony-5-upgrade-continued branch January 24, 2023 08:52
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.

3 participants