Skip to content

Use verify-hardcoded-version pre-commit hook#7726

Merged
rapids-bot[bot] merged 10 commits intorapidsai:mainfrom
KyleFromNVIDIA:verify-hardcoded-version
Feb 3, 2026
Merged

Use verify-hardcoded-version pre-commit hook#7726
rapids-bot[bot] merged 10 commits intorapidsai:mainfrom
KyleFromNVIDIA:verify-hardcoded-version

Conversation

@KyleFromNVIDIA
Copy link
Copy Markdown
Member

@KyleFromNVIDIA KyleFromNVIDIA commented Jan 27, 2026

Ops-Bot-Merge-Barrier: true

@KyleFromNVIDIA KyleFromNVIDIA requested review from a team as code owners January 27, 2026 18:41
@github-actions github-actions Bot added the Cython / Python Cython or Python issue label Jan 27, 2026
@KyleFromNVIDIA KyleFromNVIDIA added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change and removed Cython / Python Cython or Python issue labels Jan 27, 2026
@github-actions github-actions Bot added the Cython / Python Cython or Python issue label Jan 27, 2026
Copy link
Copy Markdown
Member

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Left one suggestion.

I also think the cases that needed # rapids-pre-commit-hooks: disable comments here are common enough that it would be worth adding more heuristics (expanding on rapidsai/pre-commit-hooks#102) to exclude these strings (case insensitive):

  • "in {version}"
  • "since {version}"
  • "after {version}"

I can't think of any situation where those would exist in a use-case where we'd want the output of $(cat VERSION) for {version} instead of a specific hard-coded version.

Comment thread python/cuml/cuml/internals/base.py
@KyleFromNVIDIA
Copy link
Copy Markdown
Member Author

it would be worth adding more heuristics (expanding on rapidsai/pre-commit-hooks#102) to exclude these strings (case insensitive):

Addressed in rapidsai/pre-commit-hooks#108

Copy link
Copy Markdown
Member

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Approving so you can get this merged and the repo doesn't pick up more hardcoded versions, but left a suggestion on expanding the heuristics.

Comment thread python/cuml/cuml/preprocessing/TargetEncoder.py Outdated
@github-actions github-actions Bot removed the Cython / Python Cython or Python issue label Feb 3, 2026
@KyleFromNVIDIA KyleFromNVIDIA removed the request for review from a team February 3, 2026 15:23
@KyleFromNVIDIA
Copy link
Copy Markdown
Member Author

/merge

@rapids-bot rapids-bot Bot merged commit 811afc6 into rapidsai:main Feb 3, 2026
70 checks passed
dantegd added a commit to dantegd/cuml that referenced this pull request Feb 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants