Skip to content

[GLUTEN-10834][CORE] Prefer hyphens over underscores in shell script names#10836

Merged
zhztheplayer merged 1 commit into
apache:mainfrom
Zouxxyy:dev/unify-config-name
Oct 16, 2025
Merged

[GLUTEN-10834][CORE] Prefer hyphens over underscores in shell script names#10836
zhztheplayer merged 1 commit into
apache:mainfrom
Zouxxyy:dev/unify-config-name

Conversation

@Zouxxyy

@Zouxxyy Zouxxyy commented Oct 3, 2025

Copy link
Copy Markdown
Contributor

What changes are proposed in this pull request?

use - as much as possible, unless _ is really needed for a naming in scripts, I checked the scrips in such dirs

  • .github/workflows
  • dev
  • ep
  • tools/workload

How was this patch tested?

@github-actions

github-actions Bot commented Oct 3, 2025

Copy link
Copy Markdown

#10834

@github-actions

github-actions Bot commented Oct 3, 2025

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@Zouxxyy Zouxxyy force-pushed the dev/unify-config-name branch from 783e1a1 to 0489d69 Compare October 3, 2025 01:19
@github-actions

github-actions Bot commented Oct 3, 2025

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@Zouxxyy Zouxxyy changed the title [GLUTEN-10834][CORE] Unify development script name conventions [GLUTEN-10834][CORE] Prefer - over _ in script names where possible Oct 3, 2025
@github-actions

github-actions Bot commented Oct 3, 2025

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@github-actions

github-actions Bot commented Oct 3, 2025

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@Zouxxyy Zouxxyy left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CC @zhztheplayer @philo-he , thanks

Comment thread .github/workflows/velox_weekly.yml
@Zouxxyy Zouxxyy force-pushed the dev/unify-config-name branch from 6f5d8ed to 202f9bd Compare October 3, 2025 11:41
@github-actions

github-actions Bot commented Oct 3, 2025

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@zhztheplayer

Copy link
Copy Markdown
Member

Thank you @Zouxxyy for taking this on!

@philo-he philo-he changed the title [GLUTEN-10834][CORE] Prefer - over _ in script names where possible [GLUTEN-10834][CORE] Prefer hyphens over underscores in shell script names Oct 3, 2025

@philo-he philo-he left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also felt that we should adopt a consistent naming convention.

@zhztheplayer

Copy link
Copy Markdown
Member

Thank you for the insights @philo-he. We are on the same page.

@zhztheplayer

Copy link
Copy Markdown
Member

@Zouxxyy

#10839 was merged. Would you update this one? Thanks.

@Zouxxyy Zouxxyy force-pushed the dev/unify-config-name branch from 202f9bd to d80af38 Compare October 8, 2025 13:32
@github-actions

github-actions Bot commented Oct 8, 2025

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@Zouxxyy Zouxxyy closed this Oct 9, 2025
@Zouxxyy Zouxxyy reopened this Oct 9, 2025
@github-actions

github-actions Bot commented Oct 9, 2025

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

1 similar comment
@github-actions

github-actions Bot commented Oct 9, 2025

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@Zouxxyy Zouxxyy force-pushed the dev/unify-config-name branch from 2f0c063 to 7c641d5 Compare October 9, 2025 16:02
@github-actions

github-actions Bot commented Oct 9, 2025

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@Zouxxyy

Zouxxyy commented Oct 10, 2025

Copy link
Copy Markdown
Contributor Author

I believe the failed ci was introduces after facebookincubator/velox#14849

@zhztheplayer

Copy link
Copy Markdown
Member

@Zouxxyy Would you want to rebase the PR? As I saw we have successful build in the past few days.

@Zouxxyy

Zouxxyy commented Oct 14, 2025

Copy link
Copy Markdown
Contributor Author

@zhztheplayer Will rebase once involved facebookincubator/velox#15132, the failed CI is the Velox Bachkend Weekly Job (should still fail at this point)

@Zouxxyy

Zouxxyy commented Oct 14, 2025

Copy link
Copy Markdown
Contributor Author

@zhztheplayer

Copy link
Copy Markdown
Member

Sounds good to me. Thanks.

@Zouxxyy Zouxxyy force-pushed the dev/unify-config-name branch from 7c641d5 to f509282 Compare October 16, 2025 04:39
@github-actions

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@Zouxxyy

Zouxxyy commented Oct 16, 2025

Copy link
Copy Markdown
Contributor Author

@zhztheplayer I rebased. the UT that failed previously has passed, and the current failure should be irrelevant

@philo-he philo-he left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks.

@zhztheplayer

Copy link
Copy Markdown
Member

Thanks!

@zhztheplayer zhztheplayer merged commit 1cd6031 into apache:main Oct 16, 2025
63 of 64 checks passed
meta-codesync Bot pushed a commit to facebookincubator/velox that referenced this pull request Oct 20, 2025
…led (#15113)

Summary:
In apache/gluten#10836, we encountered a Velox compilation issue caused by the changes introduced in #14849. The error occurred because ABSOLUTE_SCRIPTDIR was modified.

```
2025-10-09T03:42:02.6189401Z ++ realpath scripts
2025-10-09T03:42:02.6207867Z + ABSOLUTE_SCRIPTDIR=/__w/incubator-gluten/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/abseil-cpp/scripts
2025-10-09T03:42:02.6210432Z + VELOX_ARROW_CMAKE_PATCH=/__w/incubator-gluten/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/abseil-cpp/scripts/../CMake/resolve_dependency_modules/arrow/cmake-compatibility.patch
2025-10-09T03:42:02.6212114Z + cd /__w/incubator-gluten/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/arrow
2025-10-09T03:42:02.6213956Z + git apply /__w/incubator-gluten/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/abseil-cpp/scripts/../CMake/resolve_dependency_modules/arrow/cmake-compatibility.patch
2025-10-09T03:42:02.6230732Z error: can't open patch '/__w/incubator-gluten/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/abseil-cpp/scripts/../CMake/resolve_dependency_modules/arrow/cmake-compatibility.patch': No such file or directory
2025-10-09T03:42:02.6235097Z + exit 1
```

Pull Request resolved: #15113

Reviewed By: bikramSingh91

Differential Revision: D85060996

Pulled By: pedroerp

fbshipit-source-id: 255eb7a82bc3d9ffb6c47e2a4c96066b4974823e
mhaseeb123 pushed a commit to mhaseeb123/velox that referenced this pull request Oct 27, 2025
…led (facebookincubator#15113)

Summary:
In apache/gluten#10836, we encountered a Velox compilation issue caused by the changes introduced in facebookincubator#14849. The error occurred because ABSOLUTE_SCRIPTDIR was modified.

```
2025-10-09T03:42:02.6189401Z ++ realpath scripts
2025-10-09T03:42:02.6207867Z + ABSOLUTE_SCRIPTDIR=/__w/incubator-gluten/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/abseil-cpp/scripts
2025-10-09T03:42:02.6210432Z + VELOX_ARROW_CMAKE_PATCH=/__w/incubator-gluten/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/abseil-cpp/scripts/../CMake/resolve_dependency_modules/arrow/cmake-compatibility.patch
2025-10-09T03:42:02.6212114Z + cd /__w/incubator-gluten/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/arrow
2025-10-09T03:42:02.6213956Z + git apply /__w/incubator-gluten/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/abseil-cpp/scripts/../CMake/resolve_dependency_modules/arrow/cmake-compatibility.patch
2025-10-09T03:42:02.6230732Z error: can't open patch '/__w/incubator-gluten/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/abseil-cpp/scripts/../CMake/resolve_dependency_modules/arrow/cmake-compatibility.patch': No such file or directory
2025-10-09T03:42:02.6235097Z + exit 1
```

Pull Request resolved: facebookincubator#15113

Reviewed By: bikramSingh91

Differential Revision: D85060996

Pulled By: pedroerp

fbshipit-source-id: 255eb7a82bc3d9ffb6c47e2a4c96066b4974823e
Alan-S-Andrade pushed a commit to Alan-S-Andrade/velox that referenced this pull request Jun 2, 2026
…led (facebookincubator#15113)

Summary:
In apache/gluten#10836, we encountered a Velox compilation issue caused by the changes introduced in facebookincubator#14849. The error occurred because ABSOLUTE_SCRIPTDIR was modified.

```
2025-10-09T03:42:02.6189401Z ++ realpath scripts
2025-10-09T03:42:02.6207867Z + ABSOLUTE_SCRIPTDIR=/__w/incubator-gluten/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/abseil-cpp/scripts
2025-10-09T03:42:02.6210432Z + VELOX_ARROW_CMAKE_PATCH=/__w/incubator-gluten/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/abseil-cpp/scripts/../CMake/resolve_dependency_modules/arrow/cmake-compatibility.patch
2025-10-09T03:42:02.6212114Z + cd /__w/incubator-gluten/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/arrow
2025-10-09T03:42:02.6213956Z + git apply /__w/incubator-gluten/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/abseil-cpp/scripts/../CMake/resolve_dependency_modules/arrow/cmake-compatibility.patch
2025-10-09T03:42:02.6230732Z error: can't open patch '/__w/incubator-gluten/incubator-gluten/ep/build-velox/build/velox_ep/deps-download/abseil-cpp/scripts/../CMake/resolve_dependency_modules/arrow/cmake-compatibility.patch': No such file or directory
2025-10-09T03:42:02.6235097Z + exit 1
```

Pull Request resolved: facebookincubator#15113

Reviewed By: bikramSingh91

Differential Revision: D85060996

Pulled By: pedroerp

fbshipit-source-id: 255eb7a82bc3d9ffb6c47e2a4c96066b4974823e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants