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

Updating files to match Symfony updates #41

Closed
wants to merge 2 commits into from
Closed

Updating files to match Symfony updates #41

wants to merge 2 commits into from

Conversation

JKetelaar
Copy link

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
  1. .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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Member

@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
Copy link
Author

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

@toofff
Copy link
Contributor

toofff commented Oct 29, 2020

@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
Copy link
Author

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

@bobdenotter
Copy link
Member

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

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