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

Switch to Swagger for api documentation #6062

Merged
merged 1 commit into from Nov 16, 2022
Merged

Conversation

caspermeijn
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? How to start?
Documentation no
Translation no
CHANGELOG.md Do I just start a new section?
License MIT

I work on Read It Later wallabag client and I want better api documentation. I think the first step is updating nelmio/api-doc. This will bring an extended way to document the api.

This PR updates to nelmio/api-doc 3.0 and translates the existing documentation to the new format. I tried to keep the same content as the old api doc.

I have two outstanding issues:

  • The {_format) path parameters are not automatically shown in version 3.0. I don't see how this was added in the old version. Which api calls do have the format parameter? I assume this is only a subset of the calls, so want to add these manually.
  • nelmio/api-doc 4.0 is not possible, because is will result in a conflict. Do you see a simple way to resolve that conflict?

Makes progress on #2721

@caspermeijn caspermeijn changed the title Openapi Switch to Swagger for api documentation Nov 6, 2022
Copy link
Member

@j0k3r j0k3r left a comment

Choose a reason for hiding this comment

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

Good job!

To run test: make test
Also, run CS-Fixer too: php bin/php-cs-fixer fix

@caspermeijn
Copy link
Contributor Author

I fixed the tests and ran the formatter. I added the _format parameter to the export call, however in this version of nelmio/api-doc the {_format} is always stripped of the path. Please see if this is acceptable.

@j0k3r
Copy link
Member

j0k3r commented Nov 9, 2022

however in this version of nelmio/api-doc the {_format} is always stripped of the path

You mean, on the generated doc?

@caspermeijn
Copy link
Contributor Author

however in this version of nelmio/api-doc the {_format} is always stripped of the path

You mean, on the generated doc?

Yes, in the generated doc the {_format} is always stripped for the path. That is hard coded in the nelmio code.

@j0k3r
Copy link
Member

j0k3r commented Nov 15, 2022

I've pushed some doc update and added authorization stuff.

Here are some screenshot, before:
image

After:
image

@j0k3r j0k3r added this to the 2.6.0 milestone Nov 15, 2022
@j0k3r
Copy link
Member

j0k3r commented Nov 15, 2022

  • nelmio/api-doc 4.0 is not possible, because is will result in a conflict. Do you see a simple way to resolve that conflict?

What was that conflict?

Kdecherf
Kdecherf previously approved these changes Nov 15, 2022
Copy link
Member

@Kdecherf Kdecherf left a comment

Choose a reason for hiding this comment

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

LGTM

@caspermeijn
Copy link
Contributor Author

  • nelmio/api-doc 4.0 is not possible, because is will result in a conflict. Do you see a simple way to resolve that conflict?

What was that conflict?

composer update generates a lot of output. But I think this is main problem:

- nelmio/api-doc-bundle[v4.0.0-BETA1, ..., v4.0.1] require symfony/property-info ^4.0|^5.0 -> satisfiable by symfony/property-info[v4.0.0-BETA1, ..., 4.4.x-dev, v5.0.0-BETA1, ..., 5.4.x-dev].

@j0k3r
Copy link
Member

j0k3r commented Nov 16, 2022

We plan to upgrade to Symfony 4 later, so we'll update the bundle at that time.
Can you squash your commits? And we'll merge it. Thanks for the work!

Convert ApiDoc to Swagger
@caspermeijn
Copy link
Contributor Author

Can you squash your commits? And we'll merge it. Thanks for the work!

Done

@j0k3r j0k3r merged commit d1cdae9 into wallabag:master Nov 16, 2022
@caspermeijn caspermeijn deleted the openapi branch November 26, 2022 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants