build: add prek to run pre-commit hook for native module

[manuzhang]

Which issue does this PR close?

Closes #2927.

Rationale for this change

What changes are included in this PR?

How are these changes tested?

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.60%. Comparing base (f09f8af) to head (025d2fc).
⚠️ Report is 784 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #2951      +/-   ##
============================================
+ Coverage     56.12%   59.60%   +3.47%     
- Complexity      976     1381     +405     
============================================
  Files           119      167      +48     
  Lines         11743    15493    +3750     
  Branches       2251     2568     +317     
============================================
+ Hits           6591     9234    +2643     
- Misses         4012     4959     +947     
- Partials       1140     1300     +160     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coderfender]

I personally setup a pre commit hook but with specific shell commands to make sure that compilation , run spotless and prettier commands. That said, I don't think adding a pre commit hook at a project level might be the bestest idea :)

[manuzhang]

The gap is while checking code style is part of maven build, it is not for cargo build. Meanwhile, whether to run precommit hook locally is optional to developers with prek install.

[andygrove]

Thanks for working on this! I can see the appeal of having pre-commit hooks to catch style issues early. However, I have some concerns that make me hesitant about this approach.

My biggest concern is the external dependency. The CI now downloads and runs an installer script from a personal GitHub account (j178/prek). If that repo ever disappears or gets compromised, our CI would either break or potentially run untrusted code. For a project like Comet, I think we should be cautious about adding dependencies on tools hosted outside of well-established organizations. The direct cargo commands we had before were simple, fast, and didn't have this risk.

The diff is also quite large because of all the trailing whitespace fixes in the C files under fs-hdfs/c_src/. These look like vendored files from Hadoop, and reformatting them creates merge conflict risk with other PRs and makes it harder to diff against upstream if we ever need to update them.

There's also no documentation added for contributors explaining how to set up or use these hooks locally, so the benefit to developers isn't clear yet.

I'm wondering if the complexity and risks here outweigh the benefits. What problem are we solving that wasn't already handled by the existing cargo fmt/clippy checks in CI? If the goal is to help developers catch issues before pushing, maybe we could just document how to run those commands locally instead of adding new tooling.

---

This review was generated with AI assistance.

[manuzhang]

If the goal is to help developers catch issues before pushing, maybe we could just document how to run those commands locally instead of adding new tooling.

I'm aware of the docs but I still find myself unexpectedly committing changes with style issues. BTW, maven build has style check built-in, so I also want to make the experience consistent. Let me explore other approaches.