-
Notifications
You must be signed in to change notification settings - Fork 3.7k
GH-46794: [CI][Dev] Fix shellcheck errors in the ci/scripts/csharp_test.sh #46795
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
It seems that test failure in "C# / AMD64 macOS 13 C# 8.0.x (pull_request)" unrelated this change. https://github.com/apache/arrow/actions/runs/15611171739/job/43972190211?pr=46795#step:6:108
|
aa5c72f
to
92d6c33
Compare
I confirmed that "C# / AMD64 macOS 13 C# 8.0.x (pull_request)" error relates to this PR. Build passed without this change. I'm not sure why this change cause error yet. diff --git a/ci/scripts/csharp_test.sh b/ci/scripts/csharp_test.sh
index a435a83525..f521afc01d 100755
--- a/ci/scripts/csharp_test.sh
+++ b/ci/scripts/csharp_test.sh
@@ -30,8 +30,9 @@ if [ -z "${PYTHON}" ]; then
fi
fi
${PYTHON} -m pip install pyarrow find-libpython
-export PYTHONNET_PYDLL=$(${PYTHON} -m find_libpython)
+PYTHONNET_PYDLL=$(${PYTHON} -m find_libpython)
+export PYTHONNET_PYDLL
-pushd ${source_dir}
+pushd "${source_dir}"
dotnet test
popd |
"C# / AMD64 macOS 13 C# 8.0.x (pull_request)" succeed just added two |
This reverts commit c050cc9.
The latest version is completely the same which is previous version. CI failure last time, but now passed. C# / AMD64 macOS 13 C# 8.0.x (pull_request)C# / AMD64 macOS 13 C# 8.0.x (pull_request) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the great work you are doing with shellcheck! The previous failure seems unrelated to the change to me.
Just a small question but looks good to me.
@raulcd Thank you for your comment. Now, I'm fixing files in the After fixing all files in the Here is rough calculation per directories.
|
Thanks, great work! for file in "${files[@]}"; do
shellcheck -f diff $file | git apply
done |
@assignUser Thank you for your advice. I'll try Could someone merge this PR if this PR no problem? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit e75ecda. There were 119 benchmark results with an error:
There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
Rationale for this change
This is the sub issue #44748.
What changes are included in this PR?
"${source_dir}"
Are these changes tested?
Yes.
Are there any user-facing changes?
No.