Make JSON support optional via a feature flag (#2300)#2601
Make JSON support optional via a feature flag (#2300)#2601tustvold merged 4 commits intoapache:masterfrom
Conversation
| pub use self::data::ArrayDataRef; | ||
| pub(crate) use self::data::BufferSpec; | ||
|
|
||
| #[cfg(feature = "ipc")] |
There was a problem hiding this comment.
Drive by fix for unused code warning when compiling with --no-default-features
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Code looks good
I think we should also:
- Mention
jsonas a feature in the readme: https://github.com/apache/arrow-rs/tree/master/arrow#feature-flags - Update the CI jobs https://github.com/apache/arrow-rs/blob/master/.github/workflows/arrow.yml
Specifically for CI:
- Update clippy test to also have json feature
- Possibly also update wasm test to use json
I am not sure if there are any other jobs that used to test json but now do not.
cc @nevi-me
alamb
changed the title
alamb
changed the title
|
I think CI doesn't need updating as this feature is enabled by default? |
Several of the CI jobs explicitly run with |
Can you point me to the tests you are referring to, the ones with Edit: aah I see the wasm build is --no-default-features |
|
Yeah, I double checked -- the WASM target looks like the only one. I was mistaken about the clippy target -- it looks fine as it doesn't disable default features 👍 |
|
Benchmark runs are scheduled for baseline = 20282d1 and contender = b792ff7. b792ff7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
3 participants
Which issue does this PR close?
Closes #2300
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?