Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

improve json/xml output when params contain a single variable #91

Merged
merged 1 commit into from Nov 26, 2013

Conversation

lsmith77
Copy link
Member

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

@dbu
Copy link
Member

dbu commented Nov 26, 2013

thanks lukas!

are you sure this is no BC break? if somebody was consuming such json/xml, he will have to adjust his code, so imo this is a BC break. i agree to do this, but we should put an entry into the CHANGELOG. as this goes against the master branch, it will be in 1.1 only, which is as it should be.

@@ -90,7 +90,14 @@ public function indexAction(Request $request, $contentDocument, $contentTemplate
protected function renderResponse($contentTemplate, $params)
{
if ($this->viewHandler) {
if (count($params) === 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be 1 === count($params)

@lsmith77
Copy link
Member Author

Yes .. but at least with PHPCR it was previously impossible to serialize, but sure we can make it a 1.1 thing

@dbu
Copy link
Member

dbu commented Nov 26, 2013

ah, so it used to give an error? then lets put it in 1.0. from the title i understood it would deliver a more compact format or something ;-)
lets merge then, once you and wouter agree on the CS

@lsmith77
Copy link
Member Author

CS is fixed .. merge target is master .. so it would then also need to be cherry-picked into the 1.0 branch

lsmith77 added a commit that referenced this pull request Nov 26, 2013
improve json/xml output when params contain a single variable
@lsmith77 lsmith77 merged commit d27beb9 into master Nov 26, 2013
@lsmith77 lsmith77 deleted the tweak_fos_rest_integration branch November 26, 2013 15:55
@lsmith77
Copy link
Member Author

also cherry-picked into 1.0

@dbu
Copy link
Member

dbu commented Nov 26, 2013

thanks a lot!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants