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

MINOR: [Format] Schema.fbs grammar -- improve Map key/value field docs readability #45024

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions format/Schema.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ table FixedSizeList {
/// may be set in the metadata for this field.
///
/// In a field with Map type, the field has a child Struct field, which then
/// has two children: key type and the second the value type. The names of the
/// child fields may be respectively "entries", "key", and "value", but this is
/// not enforced.
/// has two children: the first of the key type and second of the value type.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not good at English but we don't need one more the?

Suggested change
/// has two children: the first of the key type and second of the value type.
/// has two children: the first of the key type and the second of the value type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It works grammatically either way, but the extra "the" isn't strictly necessary. Definite articles often need not be repeated in lists. I, personally, prefer the shorter form but defer to maintainers.

See https://english.stackexchange.com/questions/178767/definite-article-repetition-in-lists for a spot of context.

Copy link
Member

Choose a reason for hiding this comment

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

OK. I'll merge this PR in the next week if nobody objects this PR.

/// The names of the child fields may be respectively "entries", "key", and "value",
/// but this is not enforced.
///
/// Map
/// ```text
Expand Down
Loading