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

Adapt 'platform variable:create' to changed syntax #495

Closed
wants to merge 3 commits into from

Conversation

reithor
Copy link

@reithor reithor commented Jan 19, 2020

@reithor reithor requested a review from andrerom January 19, 2020 19:48
@andrerom andrerom requested review from damianz5 and vidarl and removed request for andrerom January 20, 2020 23:17
@lserwatka
Copy link
Member

@vidarl could you review this PR?

4. If you have the need to debug things remotely, set the `SYMFONY_ENV` environment variable to 'dev':
`platform project:variable:set env:SYMONY_ENV dev`.
`platform variable:update env:SYMONY_ENV --value 'dev'`.
Copy link
Member

@vidarl vidarl May 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit fuzzed by the use of update here.. We set the the SYMFONY_ENV in .platform.app.yaml. Have you confirmed that setting this command actually overrides that setting? Honestly, I couldn't find in p.sh docs if environment variables defined in .platform.app.yaml has precedence over project/environment variables or not.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have checked it - and it overrides the setting.
Syntax is slightly different though:
platform variable:update --value 'dev' env:SYMFONY_ENV

But simply setting SYMFONY_ENV to dev won't work anyway - in .platform.app.yaml we use composer install :
composer install --no-dev --prefer-dist --no-progress --no-interaction --optimize-autoloader
so dev packages are not installed on p.sh by default

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I know.. We had a comment about this in .platform.app.yaml earlier, but it was removed.
So, maybe add info about this in a 6.5 bullet ?

@reithor reithor closed this Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants