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: operationId should conform to naming convention #124

Merged
merged 6 commits into from
Jan 29, 2020

Conversation

SaltedCaramelCoffee
Copy link
Contributor

@SaltedCaramelCoffee SaltedCaramelCoffee commented Jan 29, 2020

documentation for operation Id naming convention

We should add support to the validator to check for conformance to these conventions and report any non-conformance as a warning

This PR takes the following approach to check for operationId naming convention:

  • Only consider paths p for which p and p/{param} exist, p has a GET and POST path, and p/{param} has a GET, a DELETE, and a POST or PUT or PATCH.
  • For paths that satisfy this constraint, we verify that
    • the operationId for GET on p starts with "list"
    • the operationId for POST on p starts with "create" or "add"
    • the operationId for GET on p/{param} starts with "get"
    • the operationId for DELETE on p/{param} starts with "delete"
    • the operationId for POST/PUT/PATCH on p/{param} starts with "update"

@SaltedCaramelCoffee SaltedCaramelCoffee requested review from mkistler and dpopp07 and removed request for mkistler January 29, 2020 17:59
@codecov
Copy link

codecov bot commented Jan 29, 2020

Codecov Report

Merging #124 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #124   +/-   ##
=======================================
  Coverage   92.65%   92.65%           
=======================================
  Files          56       56           
  Lines        2055     2055           
=======================================
  Hits         1904     1904           
  Misses        151      151

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 81a72f8...4f0cf0f. Read the comment docs.

Copy link
Member

@dpopp07 dpopp07 left a comment

Choose a reason for hiding this comment

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

This looks great, just asking for one small change that I think makes the code more readable

Copy link
Member

@dpopp07 dpopp07 left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

Make sure you squash the commits and use the feat commit header to trigger a release

@dpopp07
Copy link
Member

dpopp07 commented Jan 29, 2020

I approved it but now noticing that Travis builds are failing - make sure you get those sorted before merging!

@SaltedCaramelCoffee SaltedCaramelCoffee merged commit 79ca9b8 into master Jan 29, 2020
dpopp07 pushed a commit that referenced this pull request Jan 29, 2020
# [0.17.0](v0.16.2...v0.17.0) (2020-01-29)

### Features

* operationId should conform to naming convention ([#124](#124)) ([79ca9b8](79ca9b8))
@dpopp07
Copy link
Member

dpopp07 commented Jan 29, 2020

🎉 This PR is included in version 0.17.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@dpopp07 dpopp07 deleted the operation-id-name-convention-check branch January 30, 2020 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants