Skip to content

[AIP-134] Add rationale on update_mask being optional #1405

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.

Sign up for GitHub

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

Open
itsStrobe wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[AIP-134] Add rationale on update_mask being optional #1405

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions aip/general/0134.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ message UpdateBookRequest {
- The field **must** be optional, and the service **must** treat an omitted
field mask as an implied field mask equivalent to all fields that are
populated (have a non-empty value).
- Rationale is that omission of `update_mask` is equivalent on it being
Copy link
Contributor

@jskeet jskeet Aug 12, 2024

Choose a reason for hiding this comment

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

Rather than include this here, I'd suggest adding a rationale section separately.

Copy link
Collaborator

@noahdietz noahdietz Aug 12, 2024

Choose a reason for hiding this comment

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

Indeed, and I'd take a glance at the guidance in AIP-8 for this section:
image

empty. Marking this field as `REQUIRED` provides no benefit to the API
and results in an inconsistent experience across services.
- Update masks **must** support a special value `*`, meaning full replacement
(the equivalent of `PUT`).
- The request message **must not** contain any other required fields, and
Expand Down