Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[PhpunitBridge] Read environment variable from superglobals#31954

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@greg0ire
Copy link
Contributor

@greg0iregreg0ire commentedJun 8, 2019•
edited by nicolas-grekas
Loading

QA
Branch?4.3
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#31857
LicenseMIT
Doc PRn/a

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

@greg0iregreg0ireforce-pushed theread-env-var-from-superglobals branch 2 times, most recently from2a955f8 toafe721eCompareJune 8, 2019 10:28
@greg0ire
Copy link
ContributorAuthor

@nicolas-grekas should I do something similar here:

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

?

@greg0iregreg0ire changed the titleRead environment variable from superglobals[PhpunitBridge] Read environment variable from superglobalsJun 8, 2019
@greg0ire
Copy link
ContributorAuthor

Please resolve#31955 first

@nicolas-grekas
Copy link
Member

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

@greg0ire
Copy link
ContributorAuthor

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 :)

@greg0iregreg0ireforce-pushed theread-env-var-from-superglobals branch fromafe721e to0e832caCompareJune 8, 2019 14:26
@greg0iregreg0ire changed the base branch from4.4 to4.3June 8, 2019 14:26
@greg0iregreg0ireforce-pushed theread-env-var-from-superglobals branch from0e832ca toe9971b5CompareJune 8, 2019 14:37
@greg0ire
Copy link
ContributorAuthor

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

@nicolas-grekas
Copy link
Member

Let's update the bootstrap file too.

@greg0iregreg0ireforce-pushed theread-env-var-from-superglobals branch frome9971b5 to936cb7dCompareJune 8, 2019 15:13
@greg0ire
Copy link
ContributorAuthor

I should really test before I push 😓

@greg0ire
Copy link
ContributorAuthor

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

@greg0iregreg0ireforce-pushed theread-env-var-from-superglobals branch 2 times, most recently frome7e3593 to72cb77aCompareJune 8, 2019 15:26
@greg0ire
Copy link
ContributorAuthor

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

@greg0iregreg0ireforce-pushed theread-env-var-from-superglobals branch from72cb77a to1fe6eb2CompareJune 9, 2019 19:56
@greg0iregreg0ire marked this pull request as ready for reviewJune 10, 2019 07:58
@nicolas-grekasnicolas-grekas added this to the4.3 milestoneJun 11, 2019
@tgalopin
Copy link
Contributor

IMHO, this is quite important to finish and release as if I understand correctly, for now there is no way to configure the SYMFONY_DEPRECATIONS_HELPER variable except by using a real env var (quite cumbersome and the documentation is very misleading on this subject if that's true). Or am I missing something?

I'd be happy to give a hand to finish this if needed.

@greg0ire
Copy link
ContributorAuthor

Thanks for proposing your help@tgalopin . To me, this could very well be finished already, the only doubts I have are sum up in my previous comment.@nicolas-grekas what do you think the next steps are regarding this?

@greg0iregreg0ireforce-pushed theread-env-var-from-superglobals branch from1fe6eb2 to8dea1eeCompareJune 23, 2019 15:20
@greg0ire
Copy link
ContributorAuthor

I do not reproduce the build failure locally (I use php 7.3). They are probably due to autoloading conflicts, as usual.

@fabpot
Copy link
Member

Can you have a look at the broken tests?

@nicolas-grekasnicolas-grekasforce-pushed theread-env-var-from-superglobals branch 2 times, most recently frombe0ecf5 to3945785CompareJune 26, 2019 09:16
The Dotenv component has recently been switched to using superglobalsinstead of putenv(). Let us support both and give priority tosuperglobals.Closessymfony#31857
@nicolas-grekasnicolas-grekasforce-pushed theread-env-var-from-superglobals branch from3945785 to88cfcb5CompareJune 26, 2019 09:18
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

I reverted the changes onbootstrap.php:Dotenv always runs after the file is executed so that they were not needed. I also simplified the code:false already does what we planned fornull to do.
Tests will pass once this is merged up to master.

greg0ire reacted with thumbs up emoji
@nicolas-grekas
Copy link
Member

Thank you@greg0ire.

tgalopin and greg0ire reacted with hooray emoji

@nicolas-grekasnicolas-grekas merged commit88cfcb5 intosymfony:4.3Jun 26, 2019
nicolas-grekas added a commit that referenced this pull requestJun 26, 2019
…s (greg0ire)This PR was merged into the 4.3 branch.Discussion----------[PhpunitBridge] Read environment variable from superglobals| 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/aThe Dotenv component has recently been switched to using superglobalsinstead of putenv(). Let us support both and give priority tosuperglobals.Commits-------88cfcb5 [PhpunitBridge] Read environment variable from superglobals
@greg0iregreg0ire deleted the read-env-var-from-superglobals branchJune 26, 2019 09:43
@fabpotfabpot mentioned this pull requestJun 26, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

6 participants

@greg0ire@nicolas-grekas@tgalopin@fabpot@xabbuh@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp