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(update): introduce "path" variable #210

Merged
merged 4 commits into from
Aug 23, 2024

Conversation

toumorokoshi
Copy link
Member

@toumorokoshi toumorokoshi commented Aug 21, 2024

In protobuf, the resource path is only an input parameter in the context of update. This has resulted in increased complexity, such as the introduction of an IDENTIFIER field behavior that must be special-cased by linters and generators.

Normalizing update to the same pattern fixes this issue.

related to #181

🍱 Types of changes

What types of changes does your code introduce to AEP? Put an x in the boxes
that apply

  • Enhancement
  • New proposal
  • Migrated from google.aip.dev
  • Chore / Quick Fix

📋 Your checklist for this pull request

Please review the AEP Style and Guidance for
contributing to this repository.

General

In protobuf, the resource path is only an input parameter in the
context of update. This has resulted in increased complexity, such as
the introduction of an `IDENTIFIER` field behavior that must be
special-cased by linters and generators.

Normalizing update to the same pattern fixes this issue.

related to aep-dev#181
@toumorokoshi toumorokoshi requested a review from a team as a code owner August 21, 2024 04:49
toumorokoshi added a commit to aep-dev/aepc that referenced this pull request Aug 23, 2024
To ensure full adherence with the AEPs, fix all AEP-linter errors.

This resolves almost all AEP-linter errors, sans a change to 
introduce a path variable for Update which is being discussed
in aep-dev/aeps#210. 

One other error remains, which has to be do with the 
existence of an APPLY method here: aep-dev/aeps#110.

Proto package generation also now aligns with AEP best practices, and to enable
flexibility is now decoupled from the API name.
aep/general/0134/aep.md.j2 Outdated Show resolved Hide resolved
aep/general/0134/aep.md.j2 Show resolved Hide resolved
@toumorokoshi toumorokoshi merged commit 9e89b4d into aep-dev:main Aug 23, 2024
2 checks passed
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.

3 participants