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

Swagger API Documentation #1474

Merged
merged 8 commits into from Jun 14, 2020
Merged

Swagger API Documentation #1474

merged 8 commits into from Jun 14, 2020

Conversation

evantahler
Copy link
Member

@evantahler evantahler commented May 24, 2020

For years, Actionhero has shipped with a showDocumentation action that provided a way to describe the actions running on your server. The action was used to self-document the abilities of your server. However, the format of that action is arbitrary... and not helpful in a larger ecosystem.

Switching the format of this action to swagger/OpenAPI will make the Action much more useful.

  • Switch the showDocumentation action with swagger action. Remove all old references to showDocumentation
  • Include a new swagger.html and swagger action in generated projects which can consume and demo the API
  • Remove initializer/documentation, as we don't need this documentation internally without the old showDocumentation action.

Screen Shot 2020-05-23 at 5 15 28 PM

The breaking change is only around the showDocumentation and documentation initializer, which appears to not be used very much.

Closes #1470

Copy link
Member

@gcoonrod gcoonrod left a comment

Choose a reason for hiding this comment

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

Does this really need to be a breaking change?

@@ -1,87 +0,0 @@
import { api, Action } from "./../index";
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to leave this action around for backwards compatibility?

summary: action.description,
// description: action.description, // this is redundant
consumes: ["application/json"],
produces: ["application/json"],
Copy link
Member

Choose a reason for hiding this comment

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

Do our actions always consume and produce application/json?

default:
action.inputs[inputName].default !== null &&
action.inputs[inputName].default !== undefined
? typeof action.inputs[inputName].default === "object"
Copy link
Member

Choose a reason for hiding this comment

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

Typescript doesn't give us something better than typeof of === "object"?

Copy link
Member Author

Choose a reason for hiding this comment

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

not that I've found yet :(

@evantahler
Copy link
Member Author

Great questions @gcoonrod, and thanks for the review!

I think that this action is a lot like status - you are probably going to want to customize it for your application. Does you API only speak application/json? Do you want to add tags? Do you want to hide duplicate routes? No idea! I think I was trying to start with a 'common enough' boilerplate that would get folks started. That's why I like having this logic contained entirely in an action (userland) vs in an initializer deep within Actionhero that would be harder to customize.

As far as backwards-compatibility goes, I got the sense that no one was really using the documentation actions/initializers... but I'll ask again in Slack!

Comment on lines +138 to +140
securityDefinitions: {
// TODO (custom)?
},
Copy link
Member Author

Choose a reason for hiding this comment

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

@evantahler evantahler merged commit f7dbd54 into master Jun 14, 2020
@evantahler evantahler deleted the swagger branch June 14, 2020 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING-CHANGE enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ReplaceshowDocumenatation with Swagger/OpenAPI
2 participants