bolt / project

🚀 Repo to `composer create project` a Bolt 5 project.
MIT License
39 stars 38 forks source link

Updating files to match Symfony updates #41

Closed JKetelaar closed 3 years ago

JKetelaar commented 3 years ago

Current setup doesn't allow to read .env files properly

Instructions to create the bug:

  1. Remove .env from .gitignore (because we have .env.local for that)
  2. Add a .env.local to override values from .env
  3. .env has:
    DATABASE_URL=sqlite:///%kernel.project_dir%/var/data/bolt.sqlite
  4. .env.local has:
    
    DATABASE_HOST=localhost
    DATABASE_USERNAME=symfony
    DATABASE_PASSWORD=symfony
    DATABASE_NAME=symfony

DATABASE_URL=mysql://${DATABASE_USERNAME}:${DATABASE_PASSWORD}@${DATABASE_HOST}:3306/${DATABASE_NAME}



Now either running bin/console or opening the website will say either '`Malformed parameter "url"`' (referring to doctrine.yaml) or something regards to not being able to find the parameter or SQL server.

Or was there a reason to have these files specifically like they are?
bobdenotter commented 3 years ago

Hi @JKetelaar ,

Or was there a reason to have these files specifically like they are?

Mainly because Symfony 4 was the latest release when this project started, and we took it from there. In bolt/core we've already made some similar changes, see here: https://github.com/bolt/core/blob/master/public/index.php .. It's due for an update.

Did you remove declare(strict_types=1); on purpose?

Other than that, thanks for your contribution! 👍

JKetelaar commented 3 years ago

Gotcha, makes sense.

The declare strict types were removed in the latest Symfony version, thus also in this PR.

Do you mind if I also rewrite the current .env logic and use the correct way of handling .env with environments & Git/VC?

bobdenotter commented 3 years ago

Do you mind if I also rewrite the current .env logic and use the correct way of handling .env with environments & Git/VC?

By all means, go ahead! :-)

bobdenotter commented 3 years ago

@JKetelaar Not intending to rush you, but i was curious: Do you plan on adding that to this PR, or to a followup one?

JKetelaar commented 3 years ago

Oh sorry, my bad, forgot to respond. I will be doing that in a next PR

toofff commented 3 years ago

@bobdenotter I have already made this modification in my pull request #31.

I just have a difference on a line so I don't have a problem with Docker in the public/index.php file.

Here we have this:

if ($trustedProxies = $_SERVER['TRUSTED_PROXIES'] ?? false) {
    Request::setTrustedProxies(explode(',', $trustedProxies), Request::HEADER_X_FORWARDED_ALL ^ Request::HEADER_X_FORWARDED_HOST);
}

And I have this:

if ($trustedProxies = $_SERVER['TRUSTED_PROXIES'] ?? false) {
    Request::setTrustedProxies(explode(',', $trustedProxies), Request::HEADER_X_FORWARDED_ALL);
}
JKetelaar commented 3 years ago

@bobdenotter this can be closed if #31 is going to be merged

bobdenotter commented 3 years ago

We've merged #31, so I'm going to close this one. 👍