Skip to content

OCM-22785 | chore: update git hooks and add local verification scripts#3221

Merged
openshift-merge-bot[bot] merged 1 commit intoopenshift:masterfrom
olucasfreitas:OCM-22785
Mar 16, 2026
Merged

OCM-22785 | chore: update git hooks and add local verification scripts#3221
openshift-merge-bot[bot] merged 1 commit intoopenshift:masterfrom
olucasfreitas:OCM-22785

Conversation

@olucasfreitas
Copy link
Copy Markdown
Contributor

@olucasfreitas olucasfreitas commented Mar 13, 2026

PR Summary

Adds a local, fail-fast verification of the changed code

Detailed Description of the Issue

The goal here is to make sure all the necessary commands run before anything gets committed and possibly into an open PR, so we check linting, build, tests, coverage, etc, also help on test coverage.

Related Issues and PRs

  • Jira: OCM-22785
  • Fixes: N/A
  • Related PR(s): N/A
  • Related design/docs: N/A

Type of Change

  • feat - adds a new user-facing capability.
  • fix - resolves an incorrect behavior or bug.
  • docs - updates documentation only.
  • style - formatting or naming changes with no logic impact.
  • refactor - code restructuring with no behavior change.
  • test - adds or updates tests only.
  • chore - maintenance work (tooling, housekeeping, non-product code).
  • build - changes build system, packaging, or dependencies for build output.
  • ci - changes CI pipelines, jobs, or automation workflows.
  • perf - improves performance without changing intended behavior.

Previous Behavior

No local checks that assured or at least gave confidence in the committed code in certain fronts

Behavior After This Change

  • make install-hooks is the documented, explicit setup step before first commit in a clone.
  • commit message is validated before making it out of the local repo
    • blocks commit on failure/interruption
  • hack/run-checks.sh is the shared checks runner with:
    • basic and commit-message modes
    • --dry-run and --list-steps
    • fail-fast behavior with step summary
  • Changed-files coverage is enforced through make coverage-changed-files (gocovdiff, 80% threshold).
  • Redundant single-check wrapper targets were removed; native targets (make lint, make test, etc.) are the canonical path.
  • Docs updated in README.md, CONTRIBUTE.md, and tests/README.md.

How to Test (Step-by-Step)

Preconditions

  • Go toolchain installed
  • Dependencies available
  • From repo root

Test Steps

  1. Install hooks:
    • make install-hooks
  2. Validate check plan:
    • make run-checks -- basic --list-steps
    • make run-checks -- basic --dry-run
  3. Run core checks:
    • make fmt-check
    • make rosa
    • make lint
    • make test GO_TEST_FLAGS='-count=1'
    • make coverage-changed-files
  4. Validate commit message checker behavior:
    • Valid message file should pass:
      • printf 'OCM-22785 | chore: verify hook flow\n' > /tmp/valid-msg && ./hack/commit-msg-verify.sh /tmp/valid-msg
    • Invalid message file should fail:
      • printf 'bad commit title\n' > /tmp/invalid-msg && ./hack/commit-msg-verify.sh /tmp/invalid-msg

Expected Results

  • Hook installation succeeds.
  • run-checks list/dry-run outputs planned steps.
  • make rosa, make lint, make test, and make fmt-check pass.
  • make coverage-changed-files exits successfully when threshold conditions are met / no applicable changes.
  • Commit message validator accepts valid format and rejects invalid format.

Breaking Changes

  • No breaking changes
  • Yes, this PR introduces a breaking change (describe impact and migration plan below)

Breaking Change Details / Migration Plan

N/A

Developer Verification Checklist

  • Commit subject/title follows [JIRA-TICKET] | [TYPE]: <MESSAGE>.
  • PR description clearly explains both what changed and why.
  • Relevant Jira/GitHub issues and related PRs are linked.
  • Tests were added/updated where appropriate.
  • I manually tested the change.
  • make test passes.
  • make lint passes.
  • make rosa passes.
  • Documentation was added/updated where appropriate.
  • Any risk, limitation, or follow-up work is documented.

@openshift-ci openshift-ci bot requested review from jerichokeyne and tzhou5 March 13, 2026 17:37
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 13, 2026
amandahla
amandahla reviewed Mar 13, 2026
View reviewed changes
amandahla
amandahla reviewed Mar 13, 2026
View reviewed changes
amandahla
amandahla reviewed Mar 13, 2026
View reviewed changes
BraeTroutman
BraeTroutman reviewed Mar 13, 2026
View reviewed changes
amandahla
amandahla reviewed Mar 13, 2026
View reviewed changes
Copy link
Copy Markdown

@amandahla amandahla left a comment

Choose a reason for hiding this comment

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

Nice addition, left a few comments :)

amandahla
amandahla reviewed Mar 13, 2026
View reviewed changes
amandahla

This comment was marked as off-topic.

Sign in to view

@olucasfreitas
Copy link
Copy Markdown
Contributor Author

olucasfreitas commented Mar 16, 2026

/retest-required

@olucasfreitas
Copy link
Copy Markdown
Contributor Author

olucasfreitas commented Mar 16, 2026

/test e2e-presubmits-pr-rosa-hcp-advanced

amandahla
amandahla reviewed Mar 16, 2026
View reviewed changes
hack/coverage-changed-files.sh
cd "$repo_root"

required_coverage_percent="80"
gocovdiff_module="github.com/vearutop/gocovdiff"
Copy link
Copy Markdown

@amandahla amandahla Mar 16, 2026

Choose a reason for hiding this comment

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

not a blocker: I think it would be nice adding to the docs that this will be installed/used.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Mar 16, 2026

@olucasfreitas: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@BraeTroutman
Copy link
Copy Markdown
Contributor

BraeTroutman commented Mar 16, 2026

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 16, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Mar 16, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amandahla, BraeTroutman, olucasfreitas

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [BraeTroutman,olucasfreitas]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 7d54e53 into openshift:master Mar 16, 2026
12 checks passed
@olucasfreitas olucasfreitas mentioned this pull request Mar 17, 2026
Merged
23 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jerichokeyne jerichokeyne Awaiting requested review from jerichokeyne

@tzhou5 tzhou5 Awaiting requested review from tzhou5

2 more reviewers

@BraeTroutman BraeTroutman BraeTroutman left review comments

@amandahla amandahla amandahla approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@BraeTroutman BraeTroutman

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@olucasfreitas @BraeTroutman @amandahla