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

fix(AIP-156): Do not define Update on singletons with only output only fields. #1188

Merged
merged 4 commits into from
Aug 4, 2023
Merged

fix(AIP-156): Do not define Update on singletons with only output only fields. #1188

merged 4 commits into from
Aug 4, 2023

Conversation

lukesneeringer
Copy link
Contributor

@lukesneeringer lukesneeringer commented Jul 26, 2023

As discussed in the AIP meeting today. Should be non-controversial.

@lukesneeringer lukesneeringer requested a review from a team as a code owner July 26, 2023 17:23
@lukesneeringer lukesneeringer changed the title fix: Do not define Update on singletons with only output only fields. fix(AIP-156): Do not define Update on singletons with only output only fields. Jul 26, 2023
aip/general/0156.md Outdated Show resolved Hide resolved
@noahdietz noahdietz removed the request for review from hrasadi July 27, 2023 15:30
@andrei-scripniciuc
Copy link
Contributor

Could you include the context of this in the PR description as well for posterity?

@@ -57,12 +59,15 @@ rpc UpdateConfig(UpdateConfigRequest) returns (Config) {
- Singleton resources **should** define the [`Get`][aip-131] and
[`Update`][aip-134] methods, and **may** define custom methods as
appropriate.
- However, singleton resources **must not** define the [`Update`][aip-134]
Copy link
Contributor

@toumorokoshi toumorokoshi Aug 2, 2023

Choose a reason for hiding this comment

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

nit: probably should move the update guidance to it's own line (e.g. singleton resources should define update unless all fields are output-only).

Although It seems like we could just refactor this guidance to just pointing to the update method AIP in general.

@toumorokoshi toumorokoshi enabled auto-merge (squash) August 4, 2023 21:11
@toumorokoshi toumorokoshi merged commit e6d1b76 into aip-dev:master Aug 4, 2023
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