-
Notifications
You must be signed in to change notification settings - Fork 15
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
Adopt AIP-0203: Field Behavior #170
Conversation
We definitely need OpenAPI variants - I'll file an issue at least. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! A few notes on this one - really on issues in the existing AIPs*.
Co-authored-by: Yusuke Tsutsumi <[email protected]>
Co-authored-by: Yusuke Tsutsumi <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last nit, but otherwise LGTM! I'm approving assuming you'll address the one comment, but will hopefully save you the cycle time on approval.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to change "name" to "path" in a bunch of places (I think I caught them all).
After that I think we should merge this. I think this AEP will require extensive changes to generalize the guidance and then add OAS-specifics, so we should do that in a separate PR.
Co-authored-by: Mike Kistler <[email protected]>
Co-authored-by: Mike Kistler <[email protected]>
Co-authored-by: Mike Kistler <[email protected]>
Co-authored-by: Mike Kistler <[email protected]>
Co-authored-by: Mike Kistler <[email protected]>
Co-authored-by: Mike Kistler <[email protected]>
Co-authored-by: Mike Kistler <[email protected]>
Co-authored-by: Yusuke Tsutsumi <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two more minor fixes, but I'll approve as these are straightforward.
Co-authored-by: Mike Kistler <[email protected]>
Co-authored-by: Mike Kistler <[email protected]>
Fixes #29
Proto changes in aep-dev/aep-components#13
It's unclear to me if we need an OpenAPI extension, but I'd guess we do?
🍱 Types of changes
What types of changes does your code introduce to AEP? Put an
x
in the boxesthat apply
📋 Your checklist for this pull request
Please review the AEP Style and Guidance for
contributing to this repository.
General
references AEPs
correctly.
(usually
prettier -w .
)Additional checklist for a new AEP
guidelines are met.
Document structure
guidelines are met.
💝 Thank you!