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

ZMS-154 #693

Merged
merged 10 commits into from
May 30, 2024
Merged

ZMS-154 #693

merged 10 commits into from
May 30, 2024

Conversation

NickOvt
Copy link
Contributor

@NickOvt NickOvt commented May 27, 2024

Change old documentation file for the new autogenerated one

@NickOvt NickOvt requested a review from andris9 May 27, 2024 08:22
@NickOvt NickOvt self-assigned this May 27, 2024
@louis-lau
Copy link
Member

🎉

It's complete now? If you'd like I can review it in a little bit

@andris9
Copy link
Member

andris9 commented May 27, 2024

🎉

It's complete now? If you'd like I can review it in a little bit

Yes, @NickOvt has been busy with it, and finally, no more yaml wrangling by hand 🥳 All the endpoints are documented as well, even the most obscure ones.

@NickOvt
Copy link
Contributor Author

NickOvt commented May 30, 2024

@louis-lau If you have time then of course you can review it!
Overall indeed all the endpoints have been converted to be automatic, so changes should be easy and straightforward.
And also the new documentation is up-to-date with the endpoints, so they include latest request params as well as response types.

@NickOvt NickOvt merged commit 753997f into master May 30, 2024
7 checks passed
@NickOvt NickOvt deleted the ZMS-154 branch May 30, 2024 06:39
@louis-lau
Copy link
Member

On first look the spec doesn't seem to be a valid openapi spec, lots and lots of errors. I'll take a look.

@louis-lau
Copy link
Member

I've opened PR #694 and PR zone-eu/restify-api-generate#15

These should at least fix the spec so it's an openapi spec and there are no errors. There's still various warnings though. Can I leave fixing those to you @NickOvt? You should check the spec on https://editor.swagger.io/, you'll see the warnings in the sidebar. For example, this isn't really valid:

password:
  type: string
  description: New password for the account. Set to boolean false to disable password usage for the master scope, Application Specific Passwords would still be allowed
  enum:
    - false
    - ''

enum value should conform to its schema's type

Even if the type did match, the enum suggests the value here can only be false or '', which of course is not the case.


I also think we should set the operationIds manually per endpoint instead of having restify generate them automatically. We should be able to just use the old ones. For HTML docs it doesn't really matter much, but when generating api clients it matters quite a lot. It essentially determines the function name you'll be calling.

For example, with the old spec I'd be calling:

this.messagesApi.updateMessage()

In the new spec that would be:

this.messagesApi.putusersusermailboxesmailboxmessages()

I'm sure you agree that that's not very readable haha


I haven't looked at the content of the schemas etc yet, will do that later. Very excited to have this automatic generation!

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

Successfully merging this pull request may close these issues.

None yet

3 participants