Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[DI] Add a simple CSV env var processor#25627
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
I consider that the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nice :)
(fixture needs an update to make tests green) |
I don't get how to make the test green. The base64 test is green and it's almost the same. Do you have a hint? |
| { | ||
| private $parameters; | ||
| private $targetDirs = array(); | ||
| private $privates = array(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_csv_env.php+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_csv_env.php@@ -18,7 +18,11 @@ class Symfony_DI_PhpDumper_Test_CsvParameters extends Container { private $parameters; private $targetDirs = array();- private $privates = array();++ /**+ * @internal but protected for BC on cache:clear+ */+ protected $privates = array(); public function __construct() {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
But then it will fail for master right?
nicolas-grekasDec 29, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm not sure what you mean: this is for master already.
@dunglas fabbot failure here |
OH, in fact this CS fix has already been done on master AFAIK, so please just rebase to relaunch tests. |
Can we rename to |
@ostrolucky "csv_row" looks like noisy boilerplate to me. Native's |
Thank you@dunglas. |
This PR was squashed before being merged into the 4.1-dev branch (closes#25627).Discussion----------[DI] Add a simple CSV env var processor| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes <!-- don't forget to update src/**/CHANGELOG.md files -->| BC breaks? | no| Deprecations? | no <!-- don't forget to update UPGRADE-*.md files -->| Tests pass? | yes| Fixed tickets | n/a| License | MIT| Doc PR | n/aAdd a new environment variable processor to parse the very popular comma separated array format like `foo,bar`. As it uses the `str_getcsv`, it also support escaping and enclosure.I'm not sure about the name, maybe `array` or `simple_array` would be better.Commits-------d730209 [DI] Add a simple CSV env var processor
Uh oh!
There was an error while loading.Please reload this page.
Add a new environment variable processor to parse the very popular comma separated array format like
foo,bar. As it uses thestr_getcsv, it also supports escaping and enclosure.I'm not sure about the name, maybe
arrayorsimple_arraywould be better.