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 env config #11

Closed
wants to merge 1 commit into from
Closed

Add env config #11

wants to merge 1 commit into from

Conversation

CrazyBoy49z
Copy link
Contributor

No description provided.

@arukompas
Copy link
Contributor

For 99% of the cases, I don't think these ever change based on the environment. If you're accessing logs via /logs in production, you'll also access them via /logs in staging/local.

As for the other 1%, user can easily figure this out themselves and add .env variables themselves. After all, it's a config file that's published to your project files.

This change complicates the config file a bit, so I think we should skip this one for now. But thanks for contributing! 😄

@arukompas arukompas closed this Aug 23, 2022
@CrazyBoy49z
Copy link
Contributor Author

I don't know what makes it difficult for you. But not always the project can have one way. And this is complicated by the fact that you constantly have to open the editor, push changes to git to change the path. Instead of taking replace these parameters in .env

@arukompas
Copy link
Contributor

@CrazyBoy49z I understand! 😄 For developers who have different paths in different environments, they'll update the config/log-viewer.php themselves to utilise the .env variables. But that's not needed for the majority of use cases.

If we instead make this the default, then suddenly every developer has to start using .env variables on all their environments (local, staging, production, etc) instead of just editing their config/log-viewer.php once and committing.

I think it works well as it is for now. There's no solution that would make everyone happy, so let's not make any changes at this moment.

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

2 participants