Skip to content

Conversation

perdasilva
Copy link
Contributor

Description

The current bingo-upgrade make target can choke on bingo dependencies that require the whole path to the executable. For instance, golangci-lint needs to be installed with bingo as follows:

bingo get github.com/golangci/golangci-lint/cmd/golangci-lint

as opposed to:

bingo get github.com/golangci/golangci-lint

The update makes use of the 3rd column of the bingo list command:

$ bingo list
Name            Binary Name                                             Package @ Version                                                                       Build EnvVars   Build Flags
----            -----------                                             -----------------                                                                       -------------   -----------
bingo           bingo-v0.9.0                                            github.com/bwplotka/bingo@v0.9.0                                                                        
controller-gen  controller-gen-v0.16.1                                  sigs.k8s.io/controller-tools/cmd/controller-gen@v0.16.1                                                 
crd-diff        crd-diff-v0.1.0                                         github.com/everettraven/crd-diff@v0.1.0                                                                 
crd-ref-docs    crd-ref-docs-v0.1.0                                     github.com/elastic/crd-ref-docs@v0.1.0                                                                  
golangci-lint   golangci-lint-v1.61.0                                   github.com/golangci/golangci-lint/cmd/golangci-lint@v1.61.0                                             
goreleaser      goreleaser-v1.26.2                                      github.com/goreleaser/goreleaser@v1.26.2                                                                
kind            kind-v0.24.0                                            sigs.k8s.io/kind@v0.24.0                                                                                
kustomize       kustomize-v4.5.7                                        sigs.k8s.io/kustomize/kustomize/v4@v4.5.7                                                               
operator-sdk    operator-sdk-v1.36.1                                    github.com/operator-framework/operator-sdk/cmd/operator-sdk@v1.36.1                                     -ldflags=-X=github.com/operator-framework/operator-sdk/internal/version.Version=v1.34.1
opm             opm-v1.46.0                                             github.com/operator-framework/operator-registry/cmd/opm@v1.46.0                                         
setup-envtest   setup-envtest-v0.0.0-20240820183333-e6c3d139d2b6        sigs.k8s.io/controller-runtime/tools/setup-envtest@v0.0.0-20240820183333-e6c3d139d2b6  

And uses sed to strip everything after (and including) the @.

I've tested this manually and it worked for every tool.

Reviewer Checklist

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

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.48%. Comparing base (08ab7fb) to head (d7b537d).
Report is 67 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (08ab7fb) and HEAD (d7b537d). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (08ab7fb) HEAD (d7b537d)
2 1
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #417      +/-   ##
==========================================
- Coverage   85.06%   78.48%   -6.59%     
==========================================
  Files          19       31      +12     
  Lines        1346     2477    +1131     
==========================================
+ Hits         1145     1944     +799     
- Misses        125      444     +319     
- Partials       76       89      +13     

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

Copy link
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 14, 2025
@perdasilva perdasilva enabled auto-merge January 14, 2025 13:28
@perdasilva perdasilva added this pull request to the merge queue Jan 14, 2025
Merged via the queue into operator-framework:main with commit 7487d52 Jan 14, 2025
6 checks passed
@perdasilva perdasilva deleted the fix/bingo-upgrade branch January 14, 2025 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/infra area/sdk lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants