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

#Issue-367: Set SPRYKER_DEBUG_PROPEL_ENABLED depending if docker debug is set #368

Closed
wants to merge 1 commit into from
Closed

#Issue-367: Set SPRYKER_DEBUG_PROPEL_ENABLED depending if docker debug is set #368

wants to merge 1 commit into from

Conversation

dblaichinger
Copy link

@dblaichinger dblaichinger commented Sep 21, 2022

Description

Change log

  • Set SPRYKER_DEBUG_PROPEL_ENABLED environment variable depending if docker debug is set

Checklist

  • I agree with the Code Contribution License Agreement in CONTRIBUTING.md

@alexanderM91
Copy link
Collaborator

Hey @dblaichinger,
thanks a lot for you contribution.

I have a small questions, why we cannot reuse SPRYKER_DEBUG_ENABLED environment variable in the Spryker configuration as a newly introduced environment variable holds the same value and based on the same configuration?

config[PropelConstants::PROPEL_DEBUG] = (bool)getenv('SPRYKER_DEBUG_ENABLED');

@dblaichinger
Copy link
Author

Hey @alexanderM91,

thanks for your answer!

The only reason I've used a seperate env variable, because it also finds usage in the Spryker demo shops:

I'm fine with using just SPRYKER_DEBUG_ENABLED but maybe there is a reason having a separate flag for Propel debugging? Otherwise the demo shop configs might receive an update and use SPRYKER_DEBUG_ENABLED only?

@alexanderM91
Copy link
Collaborator

alexanderM91 commented Oct 4, 2022

Hey @dblaichinger,

maybe there is a reason having a separate flag for Propel debugging?

I am planning to investigate it more precisely but I don't see any reasons to use additional environment variable which is not configurable and contains the same value as SPRYKER_DEBUG_ENABLED.

Otherwise the demo shop configs might receive an update and use SPRYKER_DEBUG_ENABLED only?

Yes, you are absolutely right. Most probably it will come with the next public release.

@alexanderM91 alexanderM91 added bug Something isn't working and removed request-clarification labels Oct 17, 2022
@dblaichinger
Copy link
Author

Hi @alexanderM91,

sorry but can you please explain why this has been closed?
The default config still uses env var SPRYKER_DEBUG_PROPEL_ENABLED:

$config[PropelConstants::PROPEL_DEBUG] = (bool)getenv('SPRYKER_DEBUG_PROPEL_ENABLED');

https://github.com/spryker-shop/b2c-demo-shop/blob/master/config/Shared/config_default-docker.dev.php#L52

Or should there be an issue created at spryker-shop/b2c-demo-shop?

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

None yet

2 participants