Updated the application to use Symfony Flex #617
Conversation
config/packages/app.yaml
Outdated
| @@ -0,0 +1,25 @@ | |||
| services: | |||
There was a problem hiding this comment.
I guess this file can be removed in favor of config/services.yaml?
There was a problem hiding this comment.
Yes, I removed it ... but whenever you run composer install, the file is recreated, so I prefer to keep it until Symfony removes it officially.
| @@ -0,0 +1,7 @@ | |||
| twig: | |||
| paths: ['%kernel.project_dir%/templates'] | |||
There was a problem hiding this comment.
As soon as symfony/symfony#23339 to be merged, the default path to override bundles will be templates/bundles/, so what about omit (i.e. for templates) the src/Resources/ path to override the TwigBundle's templates? this configuration should be enough to move them:
twig:
paths:
templates: ~
templates/bundles/Twig: TwigLater, the error template can live there templates/bundles/Twig/Exception/error.html.twig. WDYT?
.env.dist
Outdated
| # Format described at http://docs.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/configuration.html#connecting-using-a-url | ||
| # For a MySQL database, use: "mysql://root@127.0.0.1:3306/symfony?charset=utf8mb4&serverVersion=5.7" | ||
| # Set "serverVersion" to your server version to avoid edge-case exceptions and extra database calls | ||
| DATABASE_URL="sqlite:///%kernel.project_dir%/var/data.db" |
There was a problem hiding this comment.
Meanwhile... we could use relative paths?
There was a problem hiding this comment.
With kernel.project_dir it's much readable I think
There was a problem hiding this comment.
Yes, the kernel.project_dir param is the right thing to do, but it doesn't work yet. Relative paths should be OK too, but I couldn't make it work. Only absolute paths worked for me. But probably (hopefully) I'm doing something wrong.
There was a problem hiding this comment.
@javiereguiluz, There is no data.db file. I think it should be sqlite:///%kernel.project_dir%/var/data/blog.sqlite.
There was a problem hiding this comment.
you are right, thanks! But it doesn't work either. Only absolute paths work for now.
There was a problem hiding this comment.
@javiereguiluz, It doesn't work because the current working dir is public. You should use this: sqlite:///../var/data/blog.sqlite. What about to change the current working dir in the front controller?
There was a problem hiding this comment.
Yes, the current working dir is different when we're executing a command and when we're accessing through the browser, so relative path shouldn't work.
| handler_id: session.handler.native_file | ||
| save_path: '%kernel.project_dir%/var/sessions/%kernel.environment%' | ||
|
|
||
| trusted_hosts: null |
There was a problem hiding this comment.
I think it's trusted_proxies the one that's deprecated: https://symfony.com/blog/fixing-the-trusted-proxies-configuration-for-symfony-3-3
.env.dist
Outdated
| ###> symfony/swiftmailer-bundle ### | ||
| # For Gmail as a transport, use: "gmail://username:password@localhost" | ||
| # To disable sending emails, use: "null://localhost" | ||
| MAILER_URL=smtp://localhost:25?encryption=tls&auth_mode=login |
There was a problem hiding this comment.
@javiereguiluz, you should quote the variable value with a single quotes. IMHO, it's better to use single quotes instead of double ones in the environment files, because double quotes don't support variable expansion and etc.
.env.dist
Outdated
| # Format described at http://docs.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/configuration.html#connecting-using-a-url | ||
| # For a MySQL database, use: "mysql://root@127.0.0.1:3306/symfony?charset=utf8mb4&serverVersion=5.7" | ||
| # Set "serverVersion" to your server version to avoid edge-case exceptions and extra database calls | ||
| DATABASE_URL="sqlite:///%kernel.project_dir%/var/data/blog.sqlite" |
There was a problem hiding this comment.
I agree with @yceruto, it's better to use relative path here. We should avoid to use container parameters in the environment files, because it makes them unshareable with others applications.
For example, I want to create bash script to back up the database:
#!/usr/bin/env bash
source .env;
mysqldump "$DATABASE_URL"; # It won't work because variable contains the %kernel.project_dir% parameter|
@javiereguiluz what exactly are the problems with |
result of executing: $ composer require --dev dama/doctrine-test-bundleUsing version ^3.0 for dama/doctrine-test-bundle Problem 1 Installation failed, reverting ./composer.json to its original content. |
|
@javiereguiluz fixed with the latest |
|
@dmaicher thanks! Now I can install it and I've added back the listener in PHPUnit config ... but it doesn't seem to be working. The first test is OK, but the second one is not, so the database is not restored: |
|
@javiereguiluz and is the bundle is also loaded like it was done before here? https://github.com/symfony/symfony-demo/pull/617/files#diff-c23da96dc8986a4cee89980f3aad30e0L54 |
|
@dmaicher I'm an idiot! Added the bundle as |
|
Typical mistakes of every symfony user lol |
|
Tests are failing on Travis: |
|
@javiereguiluz weird fail 😢 Maybe try to add |
|
Actually I don't see the command |
composer.json
Outdated
| "symfony/swiftmailer-bundle": "^3.0", | ||
| "symfony/translation": "^3.3", | ||
| "symfony/validator": "^3.3", | ||
| "symfony/var-dumper": "^3.3", |
There was a problem hiding this comment.
Why not put var-dumper to dev requirements?
composer.json
Outdated
| "App\\Tests\\": "tests/" | ||
| }, | ||
| "files": [ | ||
| "vendor/symfony/var-dumper/Resources/functions/dump.php" |
There was a problem hiding this comment.
I guess this file can be removed here, because var-dumper is now required directly.
| # into clickable links that open the given file using your favorite IDE. | ||
| # When 'ide' is set to null the file is opened in your web browser. | ||
| # See https://symfony.com/doc/current/reference/configuration/framework.html#ide | ||
| ide: ~ |
There was a problem hiding this comment.
This file uses null and ~. Imo we should only use one of them.
|
I need help. Travis is failing with this error: But the |
config/packages/security.yaml
Outdated
| # To load users from somewhere else: https://symfony.com/doc/current/security/custom_provider.html | ||
| database_users: | ||
| entity: { class: AppBundle:User, property: username } | ||
| entity: { class: App:User, property: username } |
There was a problem hiding this comment.
maybe we should use App\Entity\User there?
|
@javiereguiluz this file of the sec-checker recipe is missing: https://github.com/symfony/recipes/blob/master/sensiolabs/security-checker/4.0/config/packages/dev/security_checker.yaml#L5 |
|
🤦♂️ @gharlan that was it! Thank you. |
|
... and a million changes later, the Symfony Demo is upgraded to Symfony Flex and tests are green 🎉 🎉 🎉 Thank you all for your help! Now, please tell me more things to improve before merge. Thanks! |
.php_cs.dist
Outdated
| @@ -11,12 +11,12 @@ COMMENT; | |||
|
2851 |
|||
| $finder = PhpCsFixer\Finder::create() | |||
| ->in(__DIR__) | |||
| ->exclude('config') | |||
| ->exclude('var') | |||
| ->exclude('web/bundles') | |||
| ->exclude('web/css') | |||
| ->exclude('web/fonts') | |||
| ->exclude('web/js') | |||
composer.json
Outdated
| "file": "app/config/parameters.yml" | ||
| "symfony": { | ||
| "id": "01BP22MQ00QH5WD6NN6HHD3EX4", | ||
| "allow-contrib": false |
There was a problem hiding this comment.
Maybe set allow-contrib to true?
So if people use the demo and try to add a package/bundle which is available in contrib recipes, it will work just fine?
Just an idea, I'm not sure.
config/services.yaml
Outdated
| # classes (e.g. container parameters) so you must define those arguments explicitly | ||
| AppBundle\Command\ListUsersCommand: | ||
| App\Command\ListUsersCommand: | ||
| arguments: | ||
| $emailSender: '%app.notifications.email_sender%' |
| ->exclude('public/bundles') | ||
| ->exclude('public/css') | ||
| ->exclude('public/fonts') | ||
| ->exclude('public/js') |
There was a problem hiding this comment.
public/css, public/fonts and public/js => public/build
|
I could be wrong but I think the fixtures might need looking at. Have just cloned your fork and when attempting to load the fixtures I get the following: Think its due to the lack of a Bundle. Edit: I've just now, noticed your comment ( @javiereguiluz ) on their PR to fix this. - doctrine/DoctrineFixturesBundle#192 I can confirm the fix provided in that PR does solve the issue. |
composer.json
Outdated
| "symfony/polyfill-apcu": "^1.4", | ||
| "symfony/security-bundle": "^3.3", | ||
| "symfony/swiftmailer-bundle": "^3.0", | ||
| "symfony/templating": "^3.3", |
There was a problem hiding this comment.
you can remove the templating component (revert e8d0b8a), since the error is fixed in 3.3.7.
This fixes #616.
The application works, but I need to check tests because there were some issues with the
dama/doctrine-test-bundledependency used to run tests.When testing this locally, in your .env file, you must use an absolute file path to define the SQLite path. Otherwise you'll see this error:
This is a known limitation of DotEnv and we're working on it.