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.

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

feat(openapi): Add more type/int limitations #12783

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nickvergessen
Copy link
Member

  • Would something like this be desired @provokateurin
    Is a break like this okay?
  • Is it a good/okay idea to ease reusing as a API developer to introduce a subtype for things like CALL_FLAGS or is that confusing?

@nickvergessen nickvergessen added 2. developing feature: api 🛠️ OCS API for conversations, chats and participants labels Jul 23, 2024
@nickvergessen nickvergessen self-assigned this Jul 23, 2024
@nickvergessen nickvergessen added this to the 💙 Next Major (30) milestone Jul 23, 2024
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

This is a nice improvement, everything should be documented as strictly as possible.
It's not even a breaking change, since the API is the same and only the documentation for it has changed. To the end user this is not a problem, except for the case where they generate code from it, but then they have to deal with it themselves and it doesn't concern these changes.

@provokateurin
Copy link
Member

Also adding extra type aliases is helpful, otherwise code generators will need to come up with names and they can be ugly (and ofc then they are re-usable which also helps a lot).

@nickvergessen
Copy link
Member Author

Okay thanks, then I will check if there are more things to adjust and then start fixing false-negative psalm complaints

@provokateurin
Copy link
Member

The psalm complaints might be legit though

@nickvergessen
Copy link
Member Author

No, we just don't have get*() declare that they return the consts…
So yeah "legit" by psalm, but not "broken code".

@nickvergessen
Copy link
Member Author

Pushed the current state, but

* @method void setName(string $name)
* @method string getName()
* @method void setUrl(string $url)
* @method string getUrl()
is a bit annoying. It does not allow to specify non-empty-string and therefore psalm afterwards complains. Similar problem with all int fields

* participantType: TalkParticipantTypes,
* permissions: TalkPermissions,
* readOnly: 0|1,
* recordingConsent: 0|1|2,
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
* recordingConsent: 0|1|2,
* recordingConsent: 0|1,

lib/Model/Attendee.php Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing feature: api 🛠️ OCS API for conversations, chats and participants
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants