Skip to content

feature/ITSTYR 81 symfony6upgrade#25

Merged
cableman merged 106 commits intodevelopfrom
feature/ITSTYR-81_symfony6upgrade
Jan 13, 2025
Merged

feature/ITSTYR 81 symfony6upgrade#25
cableman merged 106 commits intodevelopfrom
feature/ITSTYR-81_symfony6upgrade

Conversation

@Onkel-Martin
Copy link
Copy Markdown
Contributor

  • updated readme open statment
  • README updated for composer v.2, updated upload command command
  • made a minor change
  • ITSTYR-81 / 1. Installed new symfony project / copied new docker files over / FIRST COMMIT
  • testing mermaid on git
  • added a Flowchart over the entities
  • updatet readme with flowchart text
  • updatet readme with flowchart text 2
  • ITSTYR-81 added makerbundle

@Onkel-Martin Onkel-Martin requested a review from rimi-itk March 9, 2023 12:52
Copy link
Copy Markdown
Contributor

@jekuaitk jekuaitk left a comment

Choose a reason for hiding this comment

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

Looks very good. Needs some cleanup after various debugging and the intense makeover/upgrade.

Comment thread composer.json Outdated
"symfony/security-bundle": "6.4.*",
"symfony/validator": "6.4.*",
"symfony/yaml": "6.4.*",
"symfonycasts/verify-email-bundle": "^1.17"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this verification of email bundle used?

Comment thread config/bundles.php Outdated
Knp\Bundle\PaginatorBundle\KnpPaginatorBundle::class => ['all' => true],
Sensio\Bundle\FrameworkExtraBundle\SensioFrameworkExtraBundle::class => ['all' => true],
Symfony\Bundle\WebProfilerBundle\WebProfilerBundle::class => ['dev' => true, 'test' => true],
SymfonyCasts\Bundle\VerifyEmail\SymfonyCastsVerifyEmailBundle::class => ['all' => true],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See comment regarding verify email bundle in composer.json

Comment thread docker-compose.server.yml
services:
phpfpm:
image: itkdev/php7.4-fpm:alpine
image: itkdev/php8.1-fpm:alpine
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could probably consider 8.3!

Comment thread migrations/Version20180424124131.php Outdated
class Version20180424124131 extends AbstractMigration
{
public function up(Schema $schema)
public function up(Schema $schema):void
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suppose our coding standards don't check migration files, but if we change old migration files we should probably modify them to be inline with how newly generated ones look. That is a space in between the colon and the return type.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Our coding standards should check all code we write, i.e. also the migrations.

public function up(Schema $schema): void
{
// this up() migration is auto-generated, please modify it to your needs
$this->addSql('CREATE TABLE test_user (id INT AUTO_INCREMENT NOT NULL, username VARCHAR(180) NOT NULL, roles JSON NOT NULL COMMENT \'(DC2Type:json)\', password VARCHAR(255) NOT NULL, UNIQUE INDEX UNIQ_IDENTIFIER_USERNAME (username), PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = InnoDB');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is test_user?

use Doctrine\DBAL\Types\Type;
use Doctrine\ORM\EntityManagerInterface;

class SystemImporter extends BaseImporter
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove extra blank lines in this file.

Comment thread src/Traits/ArchivableEntity.php Outdated
Comment on lines +10 to +14
// /**
// * @var \DateTime
// * @ORM\Column(type="datetime", nullable=true)
// */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
// /**
// * @var \DateTime
// * @ORM\Column(type="datetime", nullable=true)
// */

Comment thread src/Traits/ArchivableEntity.php Outdated
Comment on lines +32 to +33


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change

Comment on lines +46 to +48
{# <a href="{{ easyadmin_path('Answer', 'edit', { id: answer.id }) }}">#}


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
{# <a href="{{ easyadmin_path('Answer', 'edit', { id: answer.id }) }}">#}

Comment on lines 6 to 13
{% block main %}



<div class="container-fluid">


<h1>{{ 'export.headline' | trans }}</h1>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
{% block main %}
<div class="container-fluid">
<h1>{{ 'export.headline' | trans }}</h1>
{% block main %}
<div class="container-fluid">
<h1>{{ 'export.headline' | trans }}</h1>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Mr-Martin-Kristiansen, consider adding https://github.com/VincentLanglet/Twig-CS-Fixer as a development dependency to clean up the Twig templates. We use that tool in several projects, e.g. https://github.com/itk-dev/aapodwalk_api/ (see also https://github.com/search?q=org%3Aitk-dev%20vincentlanglet%2Ftwig-cs-fixer&type=code).

@cableman cableman merged commit c75bcb2 into develop Jan 13, 2025
@cableman cableman deleted the feature/ITSTYR-81_symfony6upgrade branch January 13, 2025 08:47
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.

5 participants