Skip to content

Update docs#378

Merged
smaye81 merged 5 commits intomainfrom
sayers/update_oneof_docs
Jun 12, 2025
Merged

Update docs#378
smaye81 merged 5 commits intomainfrom
sayers/update_oneof_docs

Conversation

@smaye81
Copy link
Copy Markdown
Contributor

@smaye81 smaye81 commented Jun 11, 2025

This updates the docs for the oneof rule. Courtesy of @jhump

@smaye81 smaye81 requested a review from jhump June 11, 2025 22:51
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 11, 2025

The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedJun 12, 2025, 4:31 PM

Copy link
Copy Markdown
Member

@timostamm timostamm left a comment

Choose a reason for hiding this comment

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

I suggest this clarification:

  //   1. Repeated and map fields are allowed in this validation. In a Protobuf oneof,
- //      only scalar fields are allowed.
+ //      only scalar, enum, and message fields are allowed.

Agreed, @jhump?

@jhump
Copy link
Copy Markdown
Member

jhump commented Jun 12, 2025

Agreed, @jhump?

Sure, that works. I guess I was thinking "singular" vs scalar. Enums really are scalars, so maybe "primitive" instead of scalar?

"primitive, enum, and message fields"

@timostamm
Copy link
Copy Markdown
Member

Sure, that works. I guess I was thinking "singular" vs scalar. Enums really are scalars, so maybe "primitive" instead of scalar?

"primitive, enum, and message fields"

Just went with the existing distinct terms "scalar" and "enum" in this file, and was wary of introducing another term.

But "primitive, enum, and message fields" or "singular fields" both work for me. Just think that we shouldn't exclude message fields in this sentence.

@smaye81 smaye81 merged commit e42bc75 into main Jun 12, 2025
7 checks passed
@smaye81 smaye81 deleted the sayers/update_oneof_docs branch June 12, 2025 16:56
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.

4 participants