When the nullable parameter of property is true, add the title parameter #1472
Conversation
LTaooo
changed the title
| @@ -373,6 +373,9 @@ public function jsonSerialize() | |||
| } else { | |||
| $ref['nullable'] = $data->nullable; | |||
| } | |||
| if (isset($data->title) && $data->title !== $defaultValues['title']) { | |||
There was a problem hiding this comment.
Doesn't this violate the 3.1.0 spec? There is code a few lines above that explicitly allows summary and description next to $ref, but nothing more.
There was a problem hiding this comment.
Hmm, interesting. I am not sure the specs do specify that sort of thing.
If it is valid do we need to handle other properties too apart from title?
There was a problem hiding this comment.
I'm not sure, maybe the description and summary can also be included. Now they are added to ref
There was a problem hiding this comment.
I guess we could try but there are some changes needed:
- update from master - I think I removed the
defaultValuecode - the logic to add the title needs to be in a 3.1.0 conditional path only
There was a problem hiding this comment.
Oh, I didn't mean we cannot do this, just that there are some more changes required. Or are you doing a new PR?
There was a problem hiding this comment.
I am not doing a new PR. Do you need to merge this PR? If you need, I will reopen it.
There was a problem hiding this comment.
Yeah, let's reopen. Those changes have to go 😔 bro the PR.
I should have some time in the next few days for some more detailed feedback.
|
Sorry, sometimes the simplest is to make the change myself. In #1487 I've changed the code slightly so all other properties are preserved. Doesn't change a lot in the tests, not sure how that works in real life though. If you could give the PR a try that would be great. |
Thank you very much. I tested it in my project, and it seems to be working fine. |
|
CLosing in favour of #1471 as that icludes the more generic handling of preserving properties. Thanks for your help. |
#1471