-
-
Notifications
You must be signed in to change notification settings - Fork 336
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
Bugfix Library #152
Bugfix Library #152
Conversation
nyamsprod
commented
Feb 9, 2016
- bug fix stream filter library doesn't support iconv stream filter #72
- improve public API by deprecating (get|set)EncodingFrom
- bug fix stream filter #72 - improve public API by deprecating (get|set)EncodingFrom
@@ -190,7 +190,7 @@ protected function newInstance($class, $open_mode) | |||
$csv->delimiter = $this->delimiter; | |||
$csv->enclosure = $this->enclosure; | |||
$csv->escape = $this->escape; | |||
$csv->encodingFrom = $this->encodingFrom; | |||
$csv->input_encoding = $this->input_encoding; |
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.
just curious: why snake_case instead of camelCase?
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 was in the mood for snake case :) . Seriously I did this to match $input_bom and $output_bom which are already in snake case. I should probably review all the codebase and make a clear decision between both coding style. Of note none of them are in contradiction to PSR2 if I recall but yes the code needs consistency.
I'll will check the codebase for consistency in property name after this PR is merge
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 was in the mood for snake case
😄
none of them are in contradiction to PSR2
You're right
* | ||
* DEPRECATION WARNING! This method will be removed in the next major point release | ||
* | ||
* @deprecated deprecated since version 4.1 |
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.
since version 4.1? should be 8.x.x (to be released)
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.
It's deprecated sinde 4.1, and will be removed in the next major version.
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 thought of some typo / old PR. Okay then, I trust you :)
@vlakoff indeed you are correct it is a typo and the deprecation message was changed in the master. 👍 I should point out that I had a hard time finding this discussion since the comment was made on an already closed PR |