Skip to content

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

Closed
akesling wants to merge 1 commit into
apache:mainfrom
akesling:patch-3
Closed

MINOR: [Format] Schema.fbs grammar -- improve Map key/value field docs readability#45024
akesling wants to merge 1 commit into
apache:mainfrom
akesling:patch-3

Conversation

@akesling
Copy link
Copy Markdown
Contributor

Rationale for this change

To improve a comment.

What changes are included in this PR?

Improvement to the Map key/value field docs readability.

Are these changes tested?

N/A

Are there any user-facing changes?

No

@github-actions github-actions Bot added the awaiting review Awaiting review label Dec 13, 2024
@kou kou changed the title MINOR: Schema.fbs grammar -- improve Map key/value field docs readability MINOR: [Format] Schema.fbs grammar -- improve Map key/value field docs readability Dec 18, 2024
Comment thread format/Schema.fbs
/// 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Dec 18, 2024
@github-actions
Copy link
Copy Markdown

Thank you for your contribution. Unfortunately, this pull request has been marked as stale because it has had no activity in the past 365 days. Please remove the stale label or comment below, or this PR will be closed in 14 days. Feel free to re-open this if it has been closed in error. If you do not have repository permissions to reopen the PR, please tag a maintainer.

@github-actions github-actions Bot added the Status: stale-warning Issues and PRs flagged as stale which are due to be closed if no indication otherwise label Feb 22, 2026
@kou
Copy link
Copy Markdown
Member

kou commented Feb 22, 2026

Sorry... I forgot to merge this...
#48697 fixed this. So I close this.

Anyway, thanks for your contribution.

@kou kou closed this Feb 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting changes Awaiting changes Status: stale-warning Issues and PRs flagged as stale which are due to be closed if no indication otherwise

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants