Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
base system
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
3 Jan 2015 at 05:38 UTC
Updated:
7 Aug 2015 at 23:24 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
kim.pepperComment #2
kim.pepperThis already caused one known issue #2401113: Update Zend Feed to latest stable
Comment #3
webchickDang. Seems like we're missing a bunch of test coverage here, then. :(
Comment #4
larowlanloose typing--, many of the places are implicitly cast to string already depending on usage - there was one that caused fails in the original issue we had to update.
Thanks for doing the rest.
In terms of tests - the fails in #2401113: Update Zend Feed to latest stable would indicate we are ok - the change in ZendFeed lost the implicit cast I assume?
Comment #5
kim.pepperYep. I think the additional check in zend-feed 2.3.*
if (!is_string($string) || empty($trimmed)) {is the trigger asis_stringwill be false whereas it was being implicitly cast further down.Comment #6
kim.pepperNeeds reroll after #2401113: Update Zend Feed to latest stable was committed.
Comment #7
kim.pepperRe-roll.
Comment #8
Jaesin commentedRe-roll.
Comment #10
dawehnerSo now that we have guzzle 6, this parameter no longer exists, see http://www.php-fig.org/psr/psr-7/
I guess then the casting, what we currently do is exactly the right thing we do.
Comment #12
berdirThe parameter was already removed in Guzzle 5, so nothing changed I think...
Comment #13
dawehnerOH yeah, you are right.
Are we sure that the test coverage really makes sense? We are probably casting somewhere, but not explicit to the string later?
Comment #14
berdirYes. I'm pretty sure nothing is actually broken here, all things things are covered by tests.
We're just removing a bogus argument and doing the string cast very explicitly instead of relying that it happens somewhere down the line, e.g. while writing it out to a file.
Comment #15
alexpottThis issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed efd4da3 and pushed to 8.0.x. Thanks!