Skip to content
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.

Sign up for GitHub

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

Jump to bottom

When the nullable parameter of property is true, add the title parameter #1472

Closed
wants to merge 2 commits into from
Closed

When the nullable parameter of property is true, add the title parameter #1472

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
730e562
When the nullable attribute of the property is true, add the title at…
LTaooo Sep 1, 2023
b7149dd
Merge branch 'zircote:master' into master
LTaooo Oct 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/Annotations/AbstractAnnotation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,9 @@ public function jsonSerialize()
} else {
$ref['nullable'] = $data->nullable;
}
if (isset($data->title) && $data->title !== $defaultValues['title']) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

https://spec.openapis.org/oas/v3.1.0.html#schemaObject

Copy link
Author

Choose a reason for hiding this comment

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

It is not to add title to $ref
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

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?

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure, maybe the description and summary can also be included. Now they are added to ref

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we could try but there are some changes needed:

  • update from master - I think I removed the defaultValue code
  • the logic to add the title needs to be in a 3.1.0 conditional path only

Copy link
Author

Choose a reason for hiding this comment

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

ok

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I didn't mean we cannot do this, just that there are some more changes required. Or are you doing a new PR?

Copy link
Author

Choose a reason for hiding this comment

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

I am not doing a new PR. Do you need to merge this PR? If you need, I will reopen it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, thinks

$ref['title'] = $data->title;
}
unset($data->nullable);
}
$data = (object) $ref;
Expand Down
Loading