martin-g commented on code in PR #2179:
URL:
https://github.com/apache/datafusion-ballista/pull/2179#discussion_r3655030616
##########
ballista-cli/Cargo.toml:
##########
@@ -81,25 +81,57 @@ tokio = { workspace = true, features = ["macros", "sync",
"time"] }
default = ["cli", "tui"]
cli = [
- "dep:ballista", "dep:datafusion", "dep:datafusion-cli",
- "dep:rustyline", "dep:mimalloc",
+ "dep:ballista",
+ "dep:datafusion",
+ "dep:datafusion-cli",
+ "dep:rustyline",
+ "dep:mimalloc",
Review Comment:
```suggestion
"dep:mimalloc",
"dep:rustyline",
```
##########
.github/workflows/rust.yml:
##########
@@ -238,22 +238,16 @@ jobs:
rustup toolchain install stable
rustup default stable
- uses: swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4
#2.9.1
- - name: Install cargo-tomlfmt
+ - name: Install taplo
uses: taiki-e/install-action@7572810d7dd469b651bb7793945692cf78da5dd7
# v2.85.0
with:
- tool: [email protected]
+ tool: [email protected]
Review Comment:
Why 0.9 ?
https://github.com/tamasfe/taplo/releases/tag/0.10.0 has been released 1
year ago
##########
dev/rust_lint.sh:
##########
@@ -17,9 +17,9 @@
# specific language governing permissions and limitations
# under the License.
set -e
-if ! command -v cargo-tomlfmt &> /dev/null; then
- echo "Installing cargo-tomlfmt using cargo"
- cargo install cargo-tomlfmt
+if ! command -v taplo &> /dev/null; then
+ echo "Installing taplo using cargo"
+ cargo install taplo-cli --locked
Review Comment:
👍🏻
let's do the same in `rust_toml_fmt.sh` too !
##########
ci/scripts/rust_toml_fmt.sh:
##########
@@ -17,5 +17,19 @@
# specific language governing permissions and limitations
# under the License.
-set -ex
-find . -mindepth 2 -name 'Cargo.toml' -exec cargo tomlfmt -k -p {} \;
+# Checks Cargo.toml formatting using taplo. The formatting rules live in
+# taplo.toml at the repository root.
+#
+# Usage:
+# ci/scripts/rust_toml_fmt.sh # check formatting (default, used
in CI)
+# ci/scripts/rust_toml_fmt.sh --write # reformat files in place
+
+set -e
+
Review Comment:
IMO it would be nice to auto-install it if it is not available on the system.
https://taplo.tamasfe.dev/cli/installation/binary.html
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
