Skip to content

🐛 Use $(HELM) instead of bare helm in lint-helm target - #2758

Merged
openshift-merge-bot[bot] merged 1 commit into
operator-framework:mainfrom
pedjak:lint-helm-use-pinned-helm
Jun 11, 2026
Merged

🐛 Use $(HELM) instead of bare helm in lint-helm target#2758
openshift-merge-bot[bot] merged 1 commit into
operator-framework:mainfrom
pedjak:lint-helm-use-pinned-helm

Conversation

@pedjak

@pedjakpedjak commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Description

The lint-helm Makefile target depends on $(HELM) but invoked helm directly in its recipe. This could pick up an unpinned system Helm binary (or fail if Helm isn't on PATH), which is inconsistent with how other targets use $(HELM) (e.g., line 192).

This change replaces all bare helm invocations in the lint-helm recipe with $(HELM) to use the pinned version consistently.

Addresses review comment from #2757.

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

CopilotAI review requested due to automatic review settings June 11, 2026 08:54
@netlify

netlifyBot commented Jun 11, 2026•
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for olmv1 ready!

NameLink
🔨 Latest commit3c3c70c
🔍 Latest deploy loghttps://app.netlify.com/projects/olmv1/deploys/6a2abbb3efa22a00081f2fb8
😎 Deploy Previewhttps://deploy-preview-2758--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changesRun an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

CopilotAI reviewed Jun 11, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR makes the lint-helm Makefile target consistently use the pinned Helm binary via $(HELM) instead of invoking helm directly, aligning it with other Helm-driven targets in the repository.

Changes:

  • Replace bare helm lint invocations with $(HELM) lint in lint-helm.
  • Replace bare helm template invocations with $(HELM) template in the conftest pipeline for lint-helm.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov

codecovBot commented Jun 11, 2026•
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.84%. Comparing base (9c49467) to head (3c3c70c).

Additional details and impacted files
@@ Coverage Diff @@## main #2758 +/- ##
==========================================
- Coverage 66.97% 66.84% -0.14% 
==========================================
Files 149 149 Lines 11382 11382 ==========================================
- Hits 7623 7608 -15 - Misses 3206 3218 +12 - Partials 553 556 +3
FlagCoverage Δ
e2e35.13% <ø> (+0.06%)⬆️
experimental-e2e52.34% <ø> (-0.35%)⬇️
unit52.17% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

@dtfranz

dtfranz commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ciopenshift-ciBot added the lgtm Indicates that a PR is ready to be merged. label Jun 11, 2026
rashmigottipati
rashmigottipati approved these changes Jun 11, 2026
View reviewed changes

@rashmigottipatirashmigottipati left a comment

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.

/lgtm

@pedjak
pedjakforce-pushed the lint-helm-use-pinned-helm branch from 1d2a4ff to d9b8f9fCompareJune 11, 2026 12:56
@openshift-ciopenshift-ciBot removed the lgtm Indicates that a PR is ready to be merged. label Jun 11, 2026
@openshift-ciopenshift-ciBot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 11, 2026
@perdasilva

perdasilva commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

/approve
/lgtm

@openshift-ci

openshift-ciBot commented Jun 11, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: perdasilva, rashmigottipati

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:

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

@pedjak@claude
fix: use $(HELM) instead of bare helm in lint-helm target
3c3c70c
The lint-helm target depends on $(HELM) but invoked helm directly,
which could pick up an unpinned system Helm or fail if Helm isn't
on PATH. Use $(HELM) consistently, matching other targets.
Co-Authored-By: Claude <noreply@anthropic.com>
@pedjak
pedjakforce-pushed the lint-helm-use-pinned-helm branch from d9b8f9f to 3c3c70cCompareJune 11, 2026 13:44
CopilotAI review requested due to automatic review settings June 11, 2026 13:44
@openshift-ciopenshift-ciBot removed the lgtm Indicates that a PR is ready to be merged. label Jun 11, 2026
@perdasilva

perdasilva commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ciopenshift-ciBot added the lgtm Indicates that a PR is ready to be merged. label Jun 11, 2026
CopilotAI reviewed Jun 11, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@openshift-merge-bot
openshift-merge-botBot merged commit df4bd9a into operator-framework:mainJun 11, 2026
25 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@perdasilvaperdasilvaperdasilva approved these changes
@rashmigottipatirashmigottipatirashmigottipati approved these changes
@dtfranzdtfranzAwaiting requested review from dtfranz

Assignees

@perdasilvaperdasilva

@rashmigottipatirashmigottipati

@dtfranzdtfranz

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pedjak@dtfranz@perdasilva@rashmigottipati