Skip to content

Commit f968889

Browse files
authored
refactor: enforce line width and import ordering with nightly rustfmt (#2383)
## What changes are proposed in this pull request? This PR switches our usage of `cargo fmt` to `cargo +nightly fmt` in order to leverage additional formatter features that are not yet supported in `stable`. Namely: 1. the ability to break comments at specific line width 2. Re-order imports following consistent grouping pattern Since this is only for formatting purposes, I believe using `nightly` cargo is low risk. It does not affect the actual version of Rust we compile with. **Note for reviewers:** It's a large PR but the changes are primarily generated from the formatter, key changes to review are: - `.github/workflows/build.yml` - `rustfmt.toml` (contains the specific formatting rules we desire) ## How was this change tested? Run tests and clippy
1 parent 38a1410 commit f968889

211 files changed

Lines changed: 2416 additions & 2132 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/build.yml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ env:
1010
# enforce the committed Cargo.lock. This prevents CI from silently resolving a newer
1111
# (potentially compromised) dependency version. If Cargo.lock is out of sync with
1212
# Cargo.toml, the build fails immediately. Any dependency change must be an explicit,
13-
# reviewable update to Cargo.lock in the PR. Commands that skip --locked: cargo fmt
13+
# reviewable update to Cargo.lock in the PR. Commands that skip --locked: cargo +nightly fmt
1414
# (no dep resolution), cargo msrv verify/show (wrapper tool), cargo miri setup (tooling).
1515
#
1616
# Swatinem/rust-cache caches the cargo registry and target directory (~450MB per job).
@@ -41,13 +41,14 @@ jobs:
4141
runs-on: ubuntu-latest
4242
steps:
4343
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
44-
- name: Install minimal stable with rustfmt
44+
- name: Install nightly with rustfmt
4545
uses: actions-rust-lang/setup-rust-toolchain@150fca883cd4034361b621bd4e6a9d34e5143606 # v1.15.4
4646
with:
4747
cache: false
48+
toolchain: nightly
4849
components: rustfmt
4950
- name: format
50-
run: cargo fmt -- --check
51+
run: cargo +nightly fmt -- --check
5152

5253
msrv:
5354
runs-on: ubuntu-latest

CLAUDE.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ cargo nextest run -p delta_kernel --lib --all-features test_name_here
3030
cargo nextest run --workspace --all-features test_name_here
3131

3232
# Format, lint, and doc check (always run after code changes)
33-
cargo fmt \
33+
cargo +nightly fmt \
3434
&& cargo clippy --workspace --benches --tests --all-features -- -D warnings \
3535
&& cargo doc --workspace --all-features --no-deps
3636

@@ -39,7 +39,7 @@ cargo clippy --workspace --no-default-features --features arrow \
3939
--exclude delta_kernel --exclude delta_kernel_ffi --exclude delta_kernel_derive --exclude delta_kernel_ffi_macros -- -D warnings
4040

4141
# Quick pre-push check (mimics CI)
42-
cargo fmt \
42+
cargo +nightly fmt \
4343
&& cargo clippy --workspace --benches --tests --all-features -- -D warnings \
4444
&& cargo doc --workspace --all-features --no-deps \
4545
&& cargo nextest run --workspace --all-features
@@ -226,7 +226,7 @@ and data flow. Keep it concise.
226226
a newer (potentially compromised) transitive dependency. If `Cargo.lock` is out of sync with
227227
`Cargo.toml`, the build fails immediately, forcing dependency changes to be explicit and
228228
reviewable. See the top-level comment in `build.yml` for full rationale. Commands exempt from
229-
`--locked`: `cargo fmt` (no dep resolution), `cargo msrv verify/show` (wrapper tool),
229+
`--locked`: `cargo +nightly fmt` (no dep resolution), `cargo msrv verify/show` (wrapper tool),
230230
`cargo miri setup` (tooling setup).
231231

232232
Ensure that when writing any github action you are considering safety including thinking of

CONTRIBUTING.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ Our trunk branch is named `main`. Here's the typical workflow:
9393
# build docs
9494
cargo doc --workspace --all-features
9595
# highly recommend editor that automatically formats, but in case you need to:
96-
cargo fmt
96+
cargo +nightly fmt
9797

9898
# run more tests
9999
cargo test --workspace --all-features -- --skip read_table_version_hdfs
@@ -116,7 +116,7 @@ Our trunk branch is named `main`. Here's the typical workflow:
116116
#### General Tips
117117

118118
1. When making your first PR, please read our contributor guidelines: https://github.com/delta-incubator/delta-kernel-rs/blob/main/CONTRIBUTING.md
119-
2. Run `cargo t --all-features --all-targets` to get started testing, and run `cargo fmt`.
119+
2. Run `cargo t --all-features --all-targets` to get started testing, and run `cargo +nightly fmt`.
120120
3. Ensure you have added or run the appropriate tests for your PR.
121121
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP] Your PR title ...'.
122122
5. Be sure to keep the PR description updated to reflect all changes.

acceptance/src/acceptance_workloads/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
//! expected_metadata/ # Expected add files after data skipping as Parquet files
2020
//! part-aaaaa.parquet
2121
//! part-bbbbb.parquet
22-
//!
2322
//! ```
2423
//!
2524
//! ## Expected Data Format

acceptance/src/acceptance_workloads/validation.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,18 @@
77
use std::fs::{self, File};
88
use std::path::Path;
99

10-
use delta_kernel::arrow::datatypes::Schema as ArrowSchema;
11-
use delta_kernel::engine::arrow_conversion::TryFromKernel;
12-
use itertools::Itertools;
13-
use tracing::debug;
14-
1510
use delta_kernel::arrow::array::RecordBatch;
1611
use delta_kernel::arrow::compute::concat_batches;
12+
use delta_kernel::arrow::datatypes::Schema as ArrowSchema;
13+
use delta_kernel::engine::arrow_conversion::TryFromKernel;
1714
use delta_kernel::parquet::arrow::arrow_reader::ParquetRecordBatchReaderBuilder;
1815
use delta_kernel::DeltaResult;
1916
use delta_kernel_benchmarks::models::{ReadExpected, SnapshotExpected};
20-
21-
use crate::data::assert_data_matches;
17+
use itertools::Itertools;
18+
use tracing::debug;
2219

2320
use super::workload::{ReadResult, SnapshotResult};
21+
use crate::data::assert_data_matches;
2422

2523
/// Read expected data from parquet files in expected_dir/expected_data/.
2624
fn read_expected_data(expected_dir: &Path) -> Result<RecordBatch, String> {

acceptance/src/acceptance_workloads/workload.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
33
use std::sync::Arc;
44

5-
use super::validation::{validate_read_result, validate_snapshot};
65
use delta_kernel::actions::{Metadata, Protocol};
76
use delta_kernel::arrow::array::RecordBatch;
87
use delta_kernel::arrow::compute::filter_record_batch;
@@ -17,6 +16,8 @@ use delta_kernel_benchmarks::predicate_parser::parse_predicate;
1716
use itertools::Itertools;
1817
use url::Url;
1918

19+
use super::validation::{validate_read_result, validate_snapshot};
20+
2021
/// Result of executing a read workload.
2122
#[derive(Debug)]
2223
pub struct ReadResult {

acceptance/src/data.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,20 @@
1-
use std::{path::Path, sync::Arc};
1+
use std::path::Path;
2+
use std::sync::Arc;
23

34
use delta_kernel::arrow::array::RecordBatch;
45
use delta_kernel::arrow::compute::concat_batches;
56
use delta_kernel::arrow::datatypes::{DataType, Field, Fields, Schema, SchemaRef};
67
use delta_kernel::arrow::util::pretty::pretty_format_batches;
7-
88
use delta_kernel::engine::arrow_data::EngineDataArrowExt as _;
9-
use delta_kernel::object_store::{local::LocalFileSystem, ObjectStore};
9+
use delta_kernel::object_store::local::LocalFileSystem;
10+
use delta_kernel::object_store::ObjectStore;
1011
use delta_kernel::parquet::arrow::async_reader::{
1112
ParquetObjectReader, ParquetRecordBatchStreamBuilder,
1213
};
1314
use delta_kernel::snapshot::Snapshot;
1415
use delta_kernel::{DeltaResult, Engine, Error};
15-
use futures::{stream::TryStreamExt, StreamExt};
16+
use futures::stream::TryStreamExt;
17+
use futures::StreamExt;
1618
use itertools::Itertools;
1719

1820
use crate::{TestCaseInfo, TestResult};

acceptance/src/meta.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@ use std::fs::File;
33
use std::path::{Path, PathBuf};
44
use std::sync::Arc;
55

6-
use delta_kernel::object_store::{local::LocalFileSystem, path::Path as ObjectPath, ObjectStore};
6+
use delta_kernel::object_store::local::LocalFileSystem;
7+
use delta_kernel::object_store::path::Path as ObjectPath;
8+
use delta_kernel::object_store::ObjectStore;
9+
use delta_kernel::{Engine, Error, Snapshot, Version};
710
use futures::stream::TryStreamExt;
811
use serde::{Deserialize, Serialize};
912
use url::Url;
1013

11-
use delta_kernel::{Engine, Error, Snapshot, Version};
12-
1314
#[derive(Debug, thiserror::Error)]
1415
pub enum AssertionError {
1516
#[error("Invalid test case data")]

acceptance/tests/acceptance_workloads_reader.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
66
use std::path::Path;
77

8-
use acceptance::acceptance_workloads::{workload::execute_and_validate_workload, TestCase};
8+
use acceptance::acceptance_workloads::workload::execute_and_validate_workload;
9+
use acceptance::acceptance_workloads::TestCase;
910

1011
/// Tests that cannot be executed due to test harness limitations.
1112
/// These fail at parse time or cause infrastructure issues (OOM, hang).

benchmarks/benches/workload_bench.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use std::sync::atomic::{AtomicBool, Ordering};
22
use std::sync::Arc;
33

44
use criterion::{criterion_group, criterion_main, Criterion};
5-
65
use delta_kernel_benchmarks::models::{
76
default_read_configs, ParallelScan, ReadConfig, ReadOperation, Spec,
87
};
@@ -77,7 +76,8 @@ fn run_benchmark(c: &mut Criterion, runner: &dyn WorkloadRunner, reporter: &Coun
7776

7877
fn build_read_configs(table_name: &str) -> Vec<ReadConfig> {
7978
// Choose which benchmark configurations to run for a given table
80-
// TODO: This function will take in table info to choose the appropriate configs for a given table
79+
// TODO: This function will take in table info to choose the appropriate configs for a given
80+
// table
8181
let mut configs = default_read_configs();
8282
if table_name.contains("V2Chkpt") {
8383
configs.push(ReadConfig {

0 commit comments

Comments
 (0)