You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/03/18 17:48:25 UTC
[GitHub] [arrow-datafusion] WinkerDu opened a new pull request #2033: fix a typo for ballista Cargo.toml
WinkerDu opened a new pull request #2033:
URL: https://github.com/apache/arrow-datafusion/pull/2033
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #.
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] WinkerDu commented on pull request #2033: fix a typo for ballista Cargo.toml
Posted by GitBox <gi...@apache.org>.
WinkerDu commented on pull request #2033:
URL: https://github.com/apache/arrow-datafusion/pull/2033#issuecomment-1072701769
cc @xudong963 , can you please approve running workflows?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] WinkerDu commented on pull request #2033: beautify the formatting in ballista Cargo.toml
Posted by GitBox <gi...@apache.org>.
WinkerDu commented on pull request #2033:
URL: https://github.com/apache/arrow-datafusion/pull/2033#issuecomment-1073003555
@yjshen Thank you. This idea sounds cool, I'd try this tool in CI workflows
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] yjshen commented on pull request #2033: beautify the formatting in ballista Cargo.toml
Posted by GitBox <gi...@apache.org>.
yjshen commented on pull request #2033:
URL: https://github.com/apache/arrow-datafusion/pull/2033#issuecomment-1072944435
It would be best if we could introduce tools like `cargo-tomlfmt` to fix existing problems and prevent further violations. As done for markdown in https://github.com/apache/arrow-datafusion/pull/453.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] WinkerDu edited a comment on pull request #2033: use cargo-tomlfmt to check Cargo.toml formatting in CI
Posted by GitBox <gi...@apache.org>.
WinkerDu edited a comment on pull request #2033:
URL: https://github.com/apache/arrow-datafusion/pull/2033#issuecomment-1073064548
@yjshen @xudong963
Now the Cargo.toml check is automatic and work well in my fork repo CI. Neat.
Could anybody help starting running CI workflows here?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] liukun4515 commented on pull request #2033: beautify the formatting in ballista Cargo.toml
Posted by GitBox <gi...@apache.org>.
liukun4515 commented on pull request #2033:
URL: https://github.com/apache/arrow-datafusion/pull/2033#issuecomment-1072928727
@WinkerDu Thanks for your contributions.
Can you help to check other toml files for the format in this pull request?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] houqp commented on a change in pull request #2033: use cargo-tomlfmt to check Cargo.toml formatting in CI
Posted by GitBox <gi...@apache.org>.
houqp commented on a change in pull request #2033:
URL: https://github.com/apache/arrow-datafusion/pull/2033#discussion_r830518863
##########
File path: Cargo.toml
##########
@@ -16,23 +16,9 @@
# under the License.
[workspace]
-members = [
- "datafusion",
- "datafusion-common",
- "datafusion-expr",
- "datafusion-jit",
- "datafusion-physical-expr",
- "datafusion-cli",
- "datafusion-examples",
- "datafusion-proto",
- "benchmarks",
- "ballista/rust/client",
- "ballista/rust/core",
- "ballista/rust/executor",
- "ballista/rust/scheduler",
- "ballista-examples",
+members = ["datafusion", "datafusion-common", "datafusion-expr", "datafusion-jit", "datafusion-physical-expr", "datafusion-cli", "datafusion-examples", "datafusion-proto", "benchmarks", "ballista/rust/client", "ballista/rust/core", "ballista/rust/executor", "ballista/rust/scheduler", "ballista-examples",
Review comment:
is there a way to configure the fmt check so that it allows multi-line lists? moving all entries into a single line increases the chances of conflicts and makes it harder to read too.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] xudong963 commented on pull request #2033: use cargo-tomlfmt to check Cargo.toml formatting in CI
Posted by GitBox <gi...@apache.org>.
xudong963 commented on pull request #2033:
URL: https://github.com/apache/arrow-datafusion/pull/2033#issuecomment-1073538590
Thanks @WinkerDu
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] WinkerDu commented on pull request #2033: beautify the formatting in ballista Cargo.toml
Posted by GitBox <gi...@apache.org>.
WinkerDu commented on pull request #2033:
URL: https://github.com/apache/arrow-datafusion/pull/2033#issuecomment-1072939520
@liukun4515 OK, I'd like to check later.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] liukun4515 commented on a change in pull request #2033: use cargo-tomlfmt to check Cargo.toml formatting in CI
Posted by GitBox <gi...@apache.org>.
liukun4515 commented on a change in pull request #2033:
URL: https://github.com/apache/arrow-datafusion/pull/2033#discussion_r830559057
##########
File path: .github/workflows/rust.yml
##########
@@ -432,6 +432,72 @@ jobs:
env:
CARGO_HOME: "/github/home/.cargo"
CARGO_TARGET_DIR: "/github/home/target"
+
+ cargo-toml-formatting-checks:
+ name: Check Cargo.toml formatting
+ needs: [linux-build-lib]
+ runs-on: ubuntu-latest
+ strategy:
+ matrix:
+ arch: [amd64]
+ rust: [stable]
+ container:
+ image: ${{ matrix.arch }}/rust
+ env:
+ # Disable full debug symbol generation to speed up CI build and keep memory down
+ # "1" means line tables only, which is useful for panic tracebacks.
+ RUSTFLAGS: "-C debuginfo=1"
+ steps:
+ - uses: actions/checkout@v2
+ with:
+ submodules: true
+ - name: Cache Cargo
+ uses: actions/cache@v2
+ with:
+ path: /github/home/.cargo
+ # this key equals the ones on `linux-build-lib` for re-use
+ key: cargo-cache-
+ - name: Cache Rust dependencies
+ uses: actions/cache@v2
+ with:
+ path: /github/home/target
+ # this key equals the ones on `linux-build-lib` for re-use
+ key: ${{ runner.os }}-${{ matrix.arch }}-target-cache-${{ matrix.rust }}
+ - name: Setup Rust toolchain
+ run: |
+ rustup toolchain install ${{ matrix.rust }}
+ rustup default ${{ matrix.rust }}
+ - name: Install cargo-tomlfmt
+ run: |
+ which cargo-tomlfmt || cargo install cargo-tomlfmt
+ env:
+ CARGO_HOME: "/github/home/.cargo"
+ CARGO_TARGET_DIR: "/github/home/target"
+ - name: Check Cargo.toml formatting
+ run: |
+ # if you encounter error, try rerun the command below, finally run 'git diff' to
+ # check which Cargo.toml introduces formatting violation
+ cargo tomlfmt -p Cargo.toml
+ cargo tomlfmt -p datafusion-cli/Cargo.toml
+ cargo tomlfmt -p ballista/rust/core/Cargo.toml
+ cargo tomlfmt -p ballista/rust/scheduler/Cargo.toml
+ cargo tomlfmt -p ballista/rust/executor/Cargo.toml
+ cargo tomlfmt -p ballista/rust/client/Cargo.toml
+ cargo tomlfmt -p datafusion/Cargo.toml
+ cargo tomlfmt -p datafusion/fuzz-utils/Cargo.toml
+ cargo tomlfmt -p ballista-examples/Cargo.toml
+ cargo tomlfmt -p datafusion-examples/Cargo.toml
+ cargo tomlfmt -p datafusion-expr/Cargo.toml
+ cargo tomlfmt -p datafusion-physical-expr/Cargo.toml
+ cargo tomlfmt -p benchmarks/Cargo.toml
+ cargo tomlfmt -p datafusion-common/Cargo.toml
+ cargo tomlfmt -p datafusion-proto/Cargo.toml
+ cargo tomlfmt -p datafusion-jit/Cargo.toml
Review comment:
good point
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] WinkerDu commented on pull request #2033: use cargo-tomlfmt to check Cargo.toml formatting in CI
Posted by GitBox <gi...@apache.org>.
WinkerDu commented on pull request #2033:
URL: https://github.com/apache/arrow-datafusion/pull/2033#issuecomment-1073432763
@liukun4515 PTAL
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] houqp commented on a change in pull request #2033: use cargo-tomlfmt to check Cargo.toml formatting in CI
Posted by GitBox <gi...@apache.org>.
houqp commented on a change in pull request #2033:
URL: https://github.com/apache/arrow-datafusion/pull/2033#discussion_r830518898
##########
File path: .github/workflows/rust.yml
##########
@@ -432,6 +432,72 @@ jobs:
env:
CARGO_HOME: "/github/home/.cargo"
CARGO_TARGET_DIR: "/github/home/target"
+
+ cargo-toml-formatting-checks:
+ name: Check Cargo.toml formatting
+ needs: [linux-build-lib]
+ runs-on: ubuntu-latest
+ strategy:
+ matrix:
+ arch: [amd64]
+ rust: [stable]
+ container:
+ image: ${{ matrix.arch }}/rust
+ env:
+ # Disable full debug symbol generation to speed up CI build and keep memory down
+ # "1" means line tables only, which is useful for panic tracebacks.
+ RUSTFLAGS: "-C debuginfo=1"
+ steps:
+ - uses: actions/checkout@v2
+ with:
+ submodules: true
+ - name: Cache Cargo
+ uses: actions/cache@v2
+ with:
+ path: /github/home/.cargo
+ # this key equals the ones on `linux-build-lib` for re-use
+ key: cargo-cache-
+ - name: Cache Rust dependencies
+ uses: actions/cache@v2
+ with:
+ path: /github/home/target
+ # this key equals the ones on `linux-build-lib` for re-use
+ key: ${{ runner.os }}-${{ matrix.arch }}-target-cache-${{ matrix.rust }}
+ - name: Setup Rust toolchain
+ run: |
+ rustup toolchain install ${{ matrix.rust }}
+ rustup default ${{ matrix.rust }}
+ - name: Install cargo-tomlfmt
+ run: |
+ which cargo-tomlfmt || cargo install cargo-tomlfmt
+ env:
+ CARGO_HOME: "/github/home/.cargo"
+ CARGO_TARGET_DIR: "/github/home/target"
+ - name: Check Cargo.toml formatting
+ run: |
+ # if you encounter error, try rerun the command below, finally run 'git diff' to
+ # check which Cargo.toml introduces formatting violation
+ cargo tomlfmt -p Cargo.toml
+ cargo tomlfmt -p datafusion-cli/Cargo.toml
+ cargo tomlfmt -p ballista/rust/core/Cargo.toml
+ cargo tomlfmt -p ballista/rust/scheduler/Cargo.toml
+ cargo tomlfmt -p ballista/rust/executor/Cargo.toml
+ cargo tomlfmt -p ballista/rust/client/Cargo.toml
+ cargo tomlfmt -p datafusion/Cargo.toml
+ cargo tomlfmt -p datafusion/fuzz-utils/Cargo.toml
+ cargo tomlfmt -p ballista-examples/Cargo.toml
+ cargo tomlfmt -p datafusion-examples/Cargo.toml
+ cargo tomlfmt -p datafusion-expr/Cargo.toml
+ cargo tomlfmt -p datafusion-physical-expr/Cargo.toml
+ cargo tomlfmt -p benchmarks/Cargo.toml
+ cargo tomlfmt -p datafusion-common/Cargo.toml
+ cargo tomlfmt -p datafusion-proto/Cargo.toml
+ cargo tomlfmt -p datafusion-jit/Cargo.toml
Review comment:
how about we use the `find` command to automatically discover all Cargo.toml files so we won't need to manually maintain this list?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] yjshen merged pull request #2033: use cargo-tomlfmt to check Cargo.toml formatting in CI
Posted by GitBox <gi...@apache.org>.
yjshen merged pull request #2033:
URL: https://github.com/apache/arrow-datafusion/pull/2033
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] WinkerDu commented on pull request #2033: use cargo-tomlfmt to check Cargo.toml formatting in CI
Posted by GitBox <gi...@apache.org>.
WinkerDu commented on pull request #2033:
URL: https://github.com/apache/arrow-datafusion/pull/2033#issuecomment-1073064548
@yjshen @xudong963
Now the Cargo.toml check is automatic and work well in my fork repo CI. Neat.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org