Skip to content

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

Merged
zhztheplayer merged 1 commit intoapache: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 intoapache:mainfrom
Zouxxyy:dev/unify-config-name

Conversation

@Zouxxyy
Copy link
Contributor

@Zouxxyy Zouxxyy commented Oct 3, 2025

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
Copy link

github-actions bot commented Oct 3, 2025

#10834

@github-actions
Copy link

github-actions bot commented Oct 3, 2025

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
Copy link

github-actions bot commented Oct 3, 2025

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
Copy link

github-actions bot commented Oct 3, 2025

Run Gluten Clickhouse CI on x86

@github-actions
Copy link

github-actions bot commented Oct 3, 2025

Run Gluten Clickhouse CI on x86

Copy link
Contributor Author

@Zouxxyy Zouxxyy left a comment

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

@Zouxxyy Zouxxyy force-pushed the dev/unify-config-name branch from 6f5d8ed to 202f9bd Compare October 3, 2025 11:41
@github-actions
Copy link

github-actions bot commented Oct 3, 2025

Run Gluten Clickhouse CI on x86

@zhztheplayer
Copy link
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
Copy link
Member

@PHILO-HE PHILO-HE left a comment

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
Member

Thank you for the insights @PHILO-HE. We are on the same page.

@zhztheplayer
Copy link
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
Copy link

github-actions bot commented Oct 8, 2025

Run Gluten Clickhouse CI on x86

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

github-actions bot commented Oct 9, 2025

Run Gluten Clickhouse CI on x86

1 similar comment
@github-actions
Copy link

github-actions bot commented Oct 9, 2025

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
Copy link

github-actions bot commented Oct 9, 2025

Run Gluten Clickhouse CI on x86

@Zouxxyy
Copy link
Contributor Author

Zouxxyy commented Oct 10, 2025

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

@zhztheplayer
Copy link
Member

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

@Zouxxyy
Copy link
Contributor Author

Zouxxyy commented Oct 14, 2025

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

@Zouxxyy
Copy link
Contributor Author

Zouxxyy commented Oct 14, 2025

@zhztheplayer
Copy link
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

Run Gluten Clickhouse CI on x86

@Zouxxyy
Copy link
Contributor Author

Zouxxyy commented Oct 16, 2025

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

Copy link
Member

@PHILO-HE PHILO-HE left a comment

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
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
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