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

PHP 8 & Symfony 6 #1153

Merged
merged 28 commits into from
Feb 8, 2025
Merged

PHP 8 & Symfony 6 #1153

merged 28 commits into from
Feb 8, 2025

Conversation

qurben
Copy link
Member

@qurben qurben commented Oct 28, 2023

Zodra de stek naar PHP 8 kan natuurlijk

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@qurben
Copy link
Member Author

qurben commented Oct 28, 2023

Ci is nog niet blij :(

@NathanHuisman NathanHuisman force-pushed the php8-symfony6 branch 2 times, most recently from 649e802 to a810a62 Compare October 20, 2024 14:58
Verander alle routing annotations naar attributes
Hierbij is ook de manier waarop de app geladen wordt veranderd,
configuratie.include.php wordt op dit moment niet geladen

TODO: We moeten deze hele logica veranderen om een beetje overeen te
komen met best practices.
@@ -102,7 +102,6 @@ package-lock.json
###< phpunit/phpunit ###

###> symfony/phpunit-bridge ###
.phpunit
Copy link
Contributor

Choose a reason for hiding this comment

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

klopt dit wel?

.idea/php.xml Outdated
@@ -222,7 +222,7 @@
<path value="$PROJECT_DIR$/vendor/twig/string-extra" />
</include_path>
</component>
<component name="PhpProjectSharedConfiguration" php_language_level="7.3">
<component name="PhpProjectSharedConfiguration" php_language_level="8.1">
Copy link
Contributor

Choose a reason for hiding this comment

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

Deze moet ook nog even naar 8.2

@@ -1,16 +1,16 @@
FROM php:7.3-apache
FROM php:8.1-apache
Copy link
Contributor

Choose a reason for hiding this comment

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

FIXME: 8.2

{
public function __construct(
private readonly EntityManagerInterface $entityManager,
private readonly ObjectNormalizer $normalizer
) {
}

public function getSupportedTypes(?string $format): array {
Copy link
Contributor

Choose a reason for hiding this comment

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

Op dit moment de meest veilige optie, ik weet niet hoe het caching mechanisme precies werkt

@@ -1,11 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<!-- https://phpunit.readthedocs.io/en/latest/configuration.html -->
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd" backupGlobals="false" colors="true" bootstrap="tests/bootstrap.php">
<coverage processUncoveredFiles="true">
Copy link
Contributor

Choose a reason for hiding this comment

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

Coverage is wel handig om te hebben, we moeten maar even kijken hoe we dat het best kunnen configureren

@@ -12,3 +12,7 @@
} elseif (method_exists(Dotenv::class, 'bootEnv')) {
(new Dotenv())->bootEnv(dirname(__DIR__) . '/.env');
}

if ($_SERVER['APP_DEBUG']) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Waar is dit voor nodig?

In plaats van een flash message, log een error.
TODO: Gebrek aan LDAP minder problematisch maken
Voeg defines.include.php toe aan require in tests/bootstrap.php
Copy link

@NathanHuisman NathanHuisman force-pushed the php8-symfony6 branch 2 times, most recently from 844d1aa to e63b46b Compare February 6, 2025 14:16
Verwijder git branch in logs, die gebruiken we toch niet
De twig alias bestond al, maar was private. Deze config werkte dus niet
waardoor het niet gegarandeerd is dat twig ook in de container zit. Fix
dit door een nieuwe publieke alias `csr.hack.twig` te maken (want het is
een hack).

Dit zou niet nodig moeten zijn en kan weggegooid worden ~wanneer~ als we de
ContainerFacade voorgoed uit de code verbannen.
Copy link

sonarqubecloud bot commented Feb 8, 2025

Copy link
Contributor

@NathanHuisman NathanHuisman left a comment

Choose a reason for hiding this comment

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

Alles is clear voor de upgrade

@NathanHuisman NathanHuisman marked this pull request as ready for review February 8, 2025 16:21
@NathanHuisman NathanHuisman merged commit a81bb3f into master Feb 8, 2025
8 of 10 checks passed
NathanHuisman added a commit that referenced this pull request Feb 10, 2025
This reverts commit a81bb3f, reversing
changes made to b599a1d.
NathanHuisman added a commit that referenced this pull request Feb 10, 2025
Revert "Merge pull request #1153 from csrdelft/php8-symfony6"
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.

2 participants