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/custom example label #1000

Merged
merged 7 commits into from Jun 30, 2021
Merged

Conversation

davidqqq
Copy link
Contributor

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you written unit tests?
  • Have you written unit tests that cover the negative cases (i.e.: if bad data is submitted, does the library respond properly)?
  • This PR is associated with an existing issue?

Closing issues

Put closes #XXXX (where XXXX is the issue number) in your comment to auto-close the issue that your PR fixes.
closes #970

If this is a new feature submission:

  • Has the issue had a maintainer respond to the issue and clarify that the feature is something that aligns with the goals and philosophy of the project?

Potential Problems With The Approach

Current solution has one limitation where space cannot be used because it will disrupt json parsing.
@example error.customError or @example error.custom_error but not @example error.custom error.

We may use "_" for space.

Test plan

Test whether label following dot has correctly been included in example object output
Test whether no or empty label has correctly been included in example object output

@@ -698,6 +698,30 @@ describe('Definition generation for OpenAPI 3.0.0', () => {
},
});
});

it('Supports custom example labels', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please add specs for OpenAPI 2? While that version does not support examples/example, it still should be clear what happens. In other cases, we add this as an x-example, I think it would be nice to add the first example without label and possibly warn about the lack of support.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get what you mean in general but not sure about the "example without label". I used an example with label in test and emit a warning if exampleLabels is not empty which should indicate usage of example labels.

/**
* @param res The alternate response
* @example res.NoSuchCountry { "errorMessage":"No such country", "errorCode": 40000 }
* @example res. { "errorMessage":"No custom label", "errorCode": 40000 }
Copy link
Collaborator

@WoH WoH Jun 5, 2021

Choose a reason for hiding this comment

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

Suggested change
* @example res. { "errorMessage":"No custom label", "errorCode": 40000 }
* @example res. { "errorMessage":"No custom label", "errorCode": 40000 }
* @example res "Unlabeled 1"
* @example res "Another unlabeled one"

Copy link
Collaborator

@WoH WoH Jun 5, 2021

Choose a reason for hiding this comment

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

Just add one or two in the middle to make sure they don't throw off the array indices.

@WoH
Copy link
Collaborator

WoH commented Jun 30, 2021

LGTM, thank you for your contribution! 🚀

@WoH WoH merged commit 7c05b4d into lukeautry:master Jun 30, 2021
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.

Custom labels for examples
2 participants