Skip to content

Commit e452cbf

Browse files
authored
fix: properly gate things by features & test for that (#1494)
Ideally we would test every combination of features, but at the very least, we should verify that no features, default features, and all features pass clippy. The two main changes: 1. In vortex-io, we need a syntactically valid expressoin when neither compio nor tokio are available. 2. In vortex-array, we need both flatbuffers and flexbuffers for views to work. I removed the optional dependencies on these libraries. In theory, someone could go through our code base and properly feature flag array views, but that began to feel onerous as I did it.
1 parent 0095a0c commit e452cbf

File tree

3 files changed

+22
-20
lines changed

3 files changed

+22
-20
lines changed

.github/workflows/ci.yml

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,20 @@ jobs:
115115
- uses: ./.github/actions/setup-rust
116116
- name: Rust Lint - Format
117117
run: cargo fmt --all --check
118-
- name: Rust Lint - Clippy
118+
- name: Rust Lint - Clippy All Features
119119
run: cargo clippy --all-features --all-targets
120+
- name: Rust Lint - Clippy Default Features
121+
run: cargo clippy --all-targets
122+
- name: Rust Lint - Clippy No Default Features
123+
run: |
124+
set -ex
125+
126+
# https://spiraldb.slack.com/archives/C07BV3GKAJ2/p1732736281946729
127+
for package in $(cargo check -p 2>&1 | grep '^ ')
128+
do
129+
echo ---- $package ----
130+
cargo clippy --package $package --no-default-features
131+
done
120132
- name: Rust Test
121133
run: cargo test --workspace --all-features
122134

vortex-array/Cargo.toml

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ arrow-select = { workspace = true }
3333
bytes = { workspace = true }
3434
enum-iterator = { workspace = true }
3535
enum-map = { workspace = true }
36-
flatbuffers = { workspace = true, optional = true }
37-
flexbuffers = { workspace = true, optional = true }
36+
flatbuffers = { workspace = true }
37+
flexbuffers = { workspace = true }
3838
futures-util = { workspace = true }
3939
hashbrown = { workspace = true }
4040
humansize = { workspace = true }
@@ -48,25 +48,13 @@ serde = { workspace = true, features = ["derive"] }
4848
static_assertions = { workspace = true }
4949
vortex-buffer = { workspace = true }
5050
vortex-datetime-dtype = { workspace = true }
51-
vortex-dtype = { workspace = true }
52-
vortex-error = { workspace = true }
53-
vortex-flatbuffers = { workspace = true, optional = true }
54-
vortex-scalar = { workspace = true }
51+
vortex-dtype = { workspace = true, features = ["flatbuffers", "serde"] }
52+
vortex-error = { workspace = true, features = ["flatbuffers", "flexbuffers"] }
53+
vortex-flatbuffers = { workspace = true, features = ["array"] }
54+
vortex-scalar = { workspace = true, features = ["flatbuffers", "serde"] }
5555

5656
[features]
57-
default = ["flatbuffers", "serde"]
5857
arbitrary = ["dep:arbitrary", "vortex-dtype/arbitrary"]
59-
flatbuffers = [
60-
"dep:flatbuffers",
61-
"dep:flexbuffers",
62-
"dep:vortex-flatbuffers",
63-
"vortex-flatbuffers/array",
64-
"vortex-dtype/flatbuffers",
65-
"vortex-error/flatbuffers",
66-
"vortex-error/flexbuffers",
67-
"vortex-scalar/flatbuffers",
68-
]
69-
serde = ["vortex-dtype/serde", "vortex-scalar/serde"]
7058

7159
[target.'cfg(target_arch = "wasm32")'.dependencies]
7260
# Enable the JS feature of getrandom (via rand) to supprt wasm32 target

vortex-io/src/dispatcher/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ mod tokio;
55
use std::future::Future;
66

77
use futures::channel::oneshot;
8+
#[cfg(not(any(feature = "compio", feature = "tokio")))]
9+
use vortex_error::vortex_panic;
810
use vortex_error::VortexResult;
911

1012
#[cfg(feature = "compio")]
@@ -73,7 +75,7 @@ impl Default for IoDispatcher {
7375
#[cfg(all(feature = "compio", not(feature = "tokio")))]
7476
return Self(Inner::Compio(CompioDispatcher::new(1)));
7577
#[cfg(not(any(feature = "compio", feature = "tokio")))]
76-
return Self(Inner {});
78+
vortex_panic!("must enable one of compio or tokio to use IoDispatcher");
7779
}
7880
}
7981

0 commit comments

Comments
 (0)