WinkerDu commented on PR #2295: URL: https://github.com/apache/arrow-datafusion/pull/2295#issuecomment-1105596532
@yjshen I run `cargo +nightly fmt -- --config format_macro_bodies=true --config format_macro_matchers=true --check` Checking result is disappointing: - No idents violation detect, such as https://github.com/apache/arrow-datafusion/blob/41b4e491663029f653e491b110d0b5e74d08a0b6/datafusion/common/src/scalar.rs#L771-L795 - Some metavariables format result is as unexpected, such as ``` Diff in /Users/duripeng/code/arrow-datafusion/arrow-datafusion/datafusion/physical-expr/src/tdigest/mod.rs at line 681: // A macro to assert the specified `quantile` estimated by `t` is within the // allowable relative error bound. macro_rules! assert_error_bounds { - ($t:ident, quantile = $quantile:literal, want = $want:literal) => { + ($t:ident,quantile = $quantile:literal,want = $want:literal) => { assert_error_bounds!( $t, quantile = $quantile, Diff in /Users/duripeng/code/arrow-datafusion/arrow-datafusion/datafusion/physical-expr/src/tdigest/mod.rs at line 689: allowable_error = 0.01 ) }; - ($t:ident, quantile = $quantile:literal, want = $want:literal, allowable_error = $re:literal) => { + ( + $t:ident,quantile = + $quantile:literal,want = + $want:literal,allowable_error = + $re:literal + ) => { let ans = $t.estimate_quantile($quantile); let expected: f64 = $want; let percentage: f64 = (expected - ans).abs() / expected; ``` IMO, it's better not to import macro format check for now, and just merge this pr for easily reading code? How do you think about it? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
