-
Notifications
You must be signed in to change notification settings - Fork 4k
GH-35698: [C#] Update FlatBuffers #35699
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
Conversation
|
|
|
@CurtHagenlocher is there still value in this PR (I'm not aware of any reason there isn't but it's been a few minutes and I just wanted to double-check before spending any significant time reviewing). |
|
@westonpace Yes, this is still worthwhile. At some point we'll want to implement whatever the final "alternate string view" design is, and being up-to-date on fbs-derived stuff will make that an easier to follow change. |
westonpace
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see now, this is just updating some vendored/generated code and there isn't much of a content update (looks like a namespace change). I agree this is a good idea. Thanks for putting this together.
|
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 0e677d2. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them. |
### Rationale for this change The FlatBuffers definitions in the C# code don't seem to have been updated for a while. Adding the latest FlatBuffers code ensures that we're up to date with both the Arrow definitions and the FlatBuffers compiler. ### What changes are included in this PR? The latest version of the C# FlatBuffers runtime files were copied into the project (with visibility changed from public to internal). The latest version of the FlatBuffers compiler was used against the latest version of the Arrow .fbs source. The output of the compiler was edited to more closely match the existing code by moving it into the directory structure and namespaces of the existing classes and by changing class visibility from public to internal. A few small changes were made to the existing source to accommodate changes in the FlatBuffers runtime, most specifically that the files have moved from the namespace "FlatBuffers" to the namespace "Google.FlatBuffers". ### Are these changes tested? No substantive product changes were made. All tests still pass. ### Are there any user-facing changes? No. Resolves apache#35698 * Closes: apache#35698 Authored-by: Curt Hagenlocher <curt@hagenlocher.org> Signed-off-by: Weston Pace <weston.pace@gmail.com>
Rationale for this change
The FlatBuffers definitions in the C# code don't seem to have been updated for a while. Adding the latest FlatBuffers code ensures that we're up to date with both the Arrow definitions and the FlatBuffers compiler.
What changes are included in this PR?
The latest version of the C# FlatBuffers runtime files were copied into the project (with visibility changed from public to internal).
The latest version of the FlatBuffers compiler was used against the latest version of the Arrow .fbs source. The output of the compiler was edited to more closely match the existing code by moving it into the directory structure and namespaces of the existing classes and by changing class visibility from public to internal.
A few small changes were made to the existing source to accommodate changes in the FlatBuffers runtime, most specifically that the files have moved from the namespace "FlatBuffers" to the namespace "Google.FlatBuffers".
Are these changes tested?
No substantive product changes were made. All tests still pass.
Are there any user-facing changes?
No.
Resolves #35698