Skip to content

Conversation

seriquynh
Copy link
Contributor

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.

@nicolas-grekas
Copy link
Member

Thank you @XuanQuynh.

@nicolas-grekas nicolas-grekas merged commit e871a6a into symfony:master Apr 11, 2019
nicolas-grekas added a commit that referenced this pull request Apr 11, 2019
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
@seriquynh seriquynh deleted the improve_dotenv_messages branch April 11, 2019 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants