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

[Dotenv] Deprecate useage of "putenv" #31062

Merged
merged 1 commit into from Apr 10, 2019

Conversation

Projects
None yet
4 participants
@Nyholm
Copy link
Member

commented Apr 10, 2019

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

From discussions on symfony/recipes#571, I think it is a good idea to make people opt-in to using putenv.

In Symfony 5.0 we will just change the value of the constructor. As an alternative, we could decide we want to remove putenv in Symfony 5.0. If so, I would also deprecate $usePutenv=true.

Show resolved Hide resolved src/Symfony/Component/Dotenv/CHANGELOG.md Outdated

@nicolas-grekas nicolas-grekas added this to the next milestone Apr 10, 2019

@nicolas-grekas
Copy link
Member

left a comment

(once comments are fixed)

@Nyholm

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2019

Thank you for the reviews

@fabpot

fabpot approved these changes Apr 10, 2019

@fabpot fabpot force-pushed the Nyholm:putenv-deprecate branch from 047c3f5 to 8e45fc0 Apr 10, 2019

@fabpot

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

Thank you @Nyholm.

@fabpot fabpot merged commit 8e45fc0 into symfony:master Apr 10, 2019

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Apr 10, 2019

feature #31062 [Dotenv] Deprecate useage of "putenv" (Nyholm)
This PR was squashed before being merged into the 4.3-dev branch (closes #31062).

Discussion
----------

[Dotenv] Deprecate useage of "putenv"

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

From discussions on symfony/recipes#571, I think it is a good idea to make people opt-in to using `putenv`.

In Symfony 5.0 we will just change the value of the constructor. As an alternative, we could decide we want to remove `putenv` in Symfony 5.0. If so, I would also deprecate `$usePutenv=true`.

Commits
-------

8e45fc0 [Dotenv] Deprecate useage of \"putenv\"

@Nyholm Nyholm deleted the Nyholm:putenv-deprecate branch Apr 10, 2019

@Nyholm

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2019

Thank you for merging

nicolas-grekas added a commit that referenced this pull request Apr 11, 2019

minor #31070 [Dotenv] Improve Dotenv messages (xuanquynh)
This PR was merged into the 4.3-dev branch.

Discussion
----------

[Dotenv] Improve Dotenv messages

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | no
| Fixed tickets | no
| License       | MIT
| Doc PR        | no

This PR improves a little bit of some messages from #31062

The first, passive sentences may be more suitable here because the value couldn't change by itself. It is changed by us - human.

The second, if we use **The default value of $usePutenv" argument of "%s\'s constructor**, we have to pass `__CLASS__` as the second parameter of `sprintf` function instead of `__METHOD__`. So, I suggest using **The default value of $usePutenv" argument of "%s"**.

Finally, the deprecation warning of `Dotenv::__construct()` is very long. Let's separate it into 2 pieces for readable reason.

Commits
-------

e871a6a Improve Dotenv messages
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.