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

Document how to use Extend for generic methods on ArrayBuilders #6932

Merged
merged 2 commits into from
Jan 4, 2025

Conversation

wiedld
Copy link
Contributor

@wiedld wiedld commented Jan 3, 2025

Which issue does this PR close?

Improve docs per this conversation, for how to use the Extend implementations of ArrayBuilders in order to build generic functions over these builders.

Rationale for this change

As noticed in this code review, where declarative macros were used instead of generics. All involved were not aware of this alternative approach of using the Extend for generic functions over the builders. Therefore, provide more docs.

What changes are included in this PR?

Docs only.

Are there any user-facing changes?

Docs only.

@github-actions github-actions bot added the arrow Changes to the arrow crate label Jan 3, 2025
@wiedld wiedld force-pushed the wiedld/doc-builder-extend-methods branch from 42a3f55 to e207ee8 Compare January 3, 2025 04:48
@wiedld wiedld marked this pull request as ready for review January 3, 2025 04:52
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @wiedld -- this is a great step. I left some suggestions -- let me know wha tyou think

I also think this is better than what is on main so we could also merge it as is

arrow-array/src/builder/mod.rs Outdated Show resolved Hide resolved
@wiedld wiedld force-pushed the wiedld/doc-builder-extend-methods branch from 644b6a1 to 4bb179b Compare January 3, 2025 18:51
@wiedld
Copy link
Contributor Author

wiedld commented Jan 3, 2025

@tlm365 -- here is the documentation on how to replace those macros with generics (if you decide to do so).

@tlm365
Copy link
Contributor

tlm365 commented Jan 4, 2025

@tlm365 -- here is the documentation on how to replace those macros with generics (if you decide to do so).

@wiedld thanks for your suggestions. I will take a look and give it a try soon (sorry, I'm a bit busy at the moment)

@tustvold tustvold merged commit 1f639f6 into apache:main Jan 4, 2025
26 checks passed
@alamb alamb deleted the wiedld/doc-builder-extend-methods branch January 4, 2025 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants