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

Add db port entry into env file. #6250

Merged

Conversation

joerose70
Copy link
Contributor

I think it's good to have db port entry in the env file so we don't have to touch the Database.config file to change the port number.

@kenjis
Copy link
Member

kenjis commented Jul 11, 2022

Thank you for contributing.

You can add port or any propertiy in Config classes to your own .env.
Why do we need to add it env?

@joerose70
Copy link
Contributor Author

I think the port setting entry is equelly important as other database-related entries in the env file like hostname, database, etc... Because any database connection requires hostname, database, username, password and port number. And even DSN records also include the port number.

Of course we can add it manully into the .env file. But it's convenient if that setting in the env file by default. So we don't have to do it manually.

@MGatner
Copy link
Member

MGatner commented Jul 11, 2022

I don't have a preference. Our example env file is intentionally not comprehensive but this seems to be the only missing database configuration field missing for a typical connection. I'd actually argue port is more likely to be changed that DBPrefix.

@joerose70
Copy link
Contributor Author

I don't have a preference. Our example env file is intentionally not comprehensive but this seems to be the only missing database configuration field missing for a typical connection. I'd actually argue port is more likely to be changed that DBPrefix.

For me having DBPrefix also very useful in the env file.

@MGatner MGatner merged commit 8d05591 into codeigniter4:develop Jul 12, 2022
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.

None yet

4 participants