Skip to content

[VL][CI] Add a format checker for scala code to enable early failure reporting#10747

Merged
philo-he merged 2 commits intoapache:mainfrom
philo-he:precheck
Sep 19, 2025
Merged

[VL][CI] Add a format checker for scala code to enable early failure reporting#10747
philo-he merged 2 commits intoapache:mainfrom
philo-he:precheck

Conversation

@philo-he
Copy link
Copy Markdown
Member

@philo-he philo-he commented Sep 18, 2025

What changes are proposed in this pull request?

Previously, code format violations were reported by the CI during scala code build, after completing the time-consuming native code build. We should perform these preliminary checks earlier to quickly surface issues, potentially saving CI resources.

How was this patch tested?

@philo-he philo-he force-pushed the precheck branch 3 times, most recently from 1d89296 to d15fd82 Compare September 18, 2025 05:35
@philo-he
Copy link
Copy Markdown
Member Author

@zhouyuan, could you take a look? This is part of preliminary checks.

@philo-he philo-he force-pushed the precheck branch 2 times, most recently from 0932d26 to 3569f34 Compare September 18, 2025 08:07
# limitations under the License.

name: Velox backend Github Runner (Enhanced Features)
name: Velox Backend (Enhanced)
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.

nit, once may not figure out the meaning of Enhanced at first glance.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@zhztheplayer, thanks for the comment. I feel this simple name is descriptive enough, especially since it's intended for developers only. Not a strong preference. I can revert to the full name if you think it's more appropriate. :)

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.

No, just keep it if you have strong reason. Never tended to be nitpicking :)

Comment thread dev/format-scala-code.sh
Comment on lines +20 to +21
PROFILES="-Pbackends-velox -Pceleborn,uniffle -Piceberg,delta,hudi,paimon \
-Pspark-3.2,spark-3.3,spark-3.4,spark-3.5,spark-4.0 -Pspark-ut"
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.

If this is not for a CI task that is only for VL backend, let's add CH's code as well.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for the comment. I note all scripts under dev/ are related to velox backend. Perhaps it’s better to continue excluding the CH backend in this script.

@philo-he
Copy link
Copy Markdown
Member Author

@zhztheplayer, could you take a look again? The CI failure has been fixed by this patch, but that job always uses upstream gluten for verification due to some known issues of checkout plugin on Centos-7.

Copy link
Copy Markdown
Member

@zhztheplayer zhztheplayer left a comment

Choose a reason for hiding this comment

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

lgtm from my end

@philo-he philo-he merged commit 24e47ff into apache:main Sep 19, 2025
62 of 63 checks passed
@philo-he philo-he deleted the precheck branch September 19, 2025 15:24
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.

2 participants