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

[PhpunitBridge] Read environment variable from superglobals #31954

Open
wants to merge 1 commit into
base: 4.3
from

Conversation

Projects
None yet
3 participants
@greg0ire
Copy link
Contributor

commented Jun 8, 2019

Q A
Branch? 4.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #31857
License MIT
Doc PR n/a

The Dotenv component has recently been switched to using superglobals
instead of putenv(). Let us support both and give priority to
superglobals.

@greg0ire greg0ire force-pushed the greg0ire:read-env-var-from-superglobals branch 2 times, most recently from 2a955f8 to afe721e Jun 8, 2019

@greg0ire

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2019

@nicolas-grekas should I do something similar here:

if ('disabled' !== getenv('SYMFONY_DEPRECATIONS_HELPER')) {
DeprecationErrorHandler::register(getenv('SYMFONY_DEPRECATIONS_HELPER'));
}

?

@greg0ire greg0ire changed the title Read environment variable from superglobals [PhpunitBridge] Read environment variable from superglobals Jun 8, 2019

@greg0ire

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2019

Please resolve #31955 first

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Jun 8, 2019

I'd propose merging this as a bug fix on 4.3, which is the very of Dotenv that relates here, WDYT?

@greg0ire

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2019

It can be viewed as a bug if you use the bridge v4.3 and a recent version of dotenv so yeah, let's do this :)

@greg0ire greg0ire force-pushed the greg0ire:read-env-var-from-superglobals branch from afe721e to 0e832ca Jun 8, 2019

@greg0ire greg0ire changed the base branch from 4.4 to 4.3 Jun 8, 2019

@greg0ire greg0ire force-pushed the greg0ire:read-env-var-from-superglobals branch from 0e832ca to e9971b5 Jun 8, 2019

@greg0ire

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2019

Done. What about my question about the bootstrap file above?

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Jun 8, 2019

Let's update the bootstrap file too.

@greg0ire greg0ire force-pushed the greg0ire:read-env-var-from-superglobals branch from e9971b5 to 936cb7d Jun 8, 2019

@greg0ire

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2019

I should really test before I push 😓

@greg0ire

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2019

Apparently having the class not defined at all is a requirement in disabled mode, so let's do this.

@greg0ire greg0ire force-pushed the greg0ire:read-env-var-from-superglobals branch 2 times, most recently from e7e3593 to 72cb77a Jun 8, 2019

@greg0ire

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2019

Is setting via putenv kind of deprecated now? Or will it be fully supported forever? Should I change existing phpt files to randomly use each of them?

Read environment variable from superglobals
The Dotenv component has recently been switched to using superglobals
instead of putenv(). Let us support both and give priority to
superglobals.

Closes #31857

@greg0ire greg0ire force-pushed the greg0ire:read-env-var-from-superglobals branch from 72cb77a to 1fe6eb2 Jun 9, 2019

@greg0ire greg0ire marked this pull request as ready for review Jun 10, 2019

@nicolas-grekas nicolas-grekas added this to the 4.3 milestone Jun 11, 2019

@nicolas-grekas nicolas-grekas added Bug and removed Feature labels Jun 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.