MINOR: [Format] Fix docstrinngs in .fbs - #50826
Open
emilk wants to merge 1 commit into
Open
Conversation
/// for doc comments so they reach generated code/// for doc comments
/// for doc comments/// for doc comments in .fbs
emilk
force-pushed
the
emilk/fbs-doc-comments
branch
from
August 6, 2026 13:33
dc73d78 to
50d2ef2
Compare
/// for doc comments in .fbs
emilk
marked this pull request as ready for review
August 6, 2026 13:33
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates FlatBuffers schema documentation comments so flatc will propagate them into generated language bindings (by converting relevant // blocks to /// and re-associating the IntervalUnit docs with the correct declarations).
Changes:
- Converted selected declaration-level comments from
//to///to ensure they are emitted in generated bindings. - Restructured
IntervalUnitdocumentation so each paragraph attaches to the intended enum/table/value. - Updated schema docs for
DurationandSchema.custom_metadatato use///.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| format/Schema.fbs | Converts and repositions doc comments to attach to IntervalUnit, Interval, Duration, and Schema.custom_metadata for codegen visibility. |
| format/Message.fbs | Converts CompressionType value docs (LZ4_FRAME, ZSTD) to /// for propagation into generated bindings. |
Suppressed comments (2)
format/Schema.fbs:424
- This comment is now user-facing; "day light savings time" is usually written as "daylight saving time".
/// A "calendar" interval which models types that don't necessarily
/// have a precise duration without the context of a base timestamp (e.g.
/// days can differ in length during day light savings time transitions).
format/Schema.fbs:441
- These lines have an extra leading space after
///(likely left over from the prior//block). Removing it will avoid odd indentation in generated docs.
/// The resolution defaults to millisecond, but can be any of the other
/// supported TimeUnit values as with Timestamp and Time types. This type is
/// always represented as an 8-byte integer.
flatc only propagates triple-slash comments into generated bindings. Several descriptions in Message.fbs and Schema.fbs use plain // and are therefore dropped. Also capitalize "Arrow" in the two prose comments that spelled it lowercase.
emilk
force-pushed
the
emilk/fbs-doc-comments
branch
from
August 6, 2026 16:46
50d2ef2 to
547a396
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
I noticed that
LZ4_FRAMElacks docs in arrow-rs and traced it back to a mistaken use of//(comment) instead of///(docstring) in a .fbs file. I decided to fix the other suspected mistakes too.Are these changes tested?
Generated Rust with
flatc 23.5.26before and after and diffed. The only differences are the new doc comment blocks.Are there any user-facing changes?
Yes! There are now docstrings in the generated code :)
Disclosure per the AI-generated code guidance: I used Claude Code to find the affected comments and verify the generated-output diff. I reviewed every line.