Skip to content

Conversation

@xmakro
Copy link
Contributor

@xmakro xmakro commented Oct 26, 2025

No description provided.

@gooroo-dev
Copy link

gooroo-dev bot commented Oct 26, 2025

Please double check the following review of the pull request:

🐞Mistake 🤪Typo 🚨Security 🚀Performance 💪Best Practices 📖Readability ❓Others
0 0 0 0 0 0 0

Changes in the diff

  • 🚀 Replaced umbrella arrow crate with finer-grained arrow-array, arrow-buffer, arrow-data, and arrow-schema subcrates to speed up compilation.
  • 📖 Updated imports throughout the codebase to use the new subcrates instead of the umbrella crate.
  • 📖 Changed error types from arrow::error::ArrowError to arrow_schema::ArrowError consistently.
  • 📖 Updated trait bounds and return types to use the new error type.
  • 📖 Reordered dependencies in Cargo.toml for clarity and added new dependencies explicitly.
  • 📖 Improved comments to reflect the new crate usage (e.g., arrow_array instead of arrow).
  • 🛠️ Minor formatting and ordering changes in Cargo.toml and code imports.

Identified Issues

No issues found. The changes are mostly mechanical refactoring to use arrow subcrates and update error types accordingly. No logic or security issues detected.

Missing Tests

No new functionality introduced that requires new tests. The changes are refactorings and dependency updates. Existing tests should cover the functionality.

Summary

The PR cleanly refactors the codebase to use arrow subcrates, which is a recommended practice to reduce compilation times. All imports and error types are updated consistently. There are no mistakes, typos, security issues, or performance regressions. The code remains clear and maintainable. No additional tests are needed.

Summon me to re-review when updated! Yours, Gooroo.dev
React or reply to give your feedback!

@xmakro
Copy link
Contributor Author

xmakro commented Nov 5, 2025

@Swoorup could you please take a look. I also wanted to send a follow up PR for upgrading to the latest arrow version

Copy link
Owner

@Swoorup Swoorup left a comment

Choose a reason for hiding this comment

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

Bit slow due to holidays, @xmakro these look good.

@Swoorup Swoorup merged commit 64362e3 into Swoorup:main Nov 6, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants