🌱 chore(lint): Upgrade golanglint from v2.7.2 to v2.8.0 and fix lint issues#1898
Conversation
.bingo/Variables.mk
Outdated
| # even when GOOS/GOARCH are set for cross-compilation of other targets. | ||
| GOHOSTOS ?= $(shell $(GO) env GOHOSTOS) | ||
| GOHOSTARCH ?= $(shell $(GO) env GOHOSTARCH) | ||
| GOHOSTARM ?= $(shell $(GO) env GOHOSTARM) |
There was a problem hiding this comment.
@tmshort bingo get github.com/golangci/golangci-lint/v2/cmd/golangci-lint here is adding those,
I remember that we cannot add those ?
I think you did something to avoid it. Could you please help me out?
Can you please point out me how to fix it?
There was a problem hiding this comment.
So it seems all is fine.
I think we cannot add those for operator-controller only doing the upstream/downstream dance.
Here shows all fine.
There was a problem hiding this comment.
Looks to be due to bingo v0.10 vs v0.9?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1898 +/- ##
=======================================
Coverage 57.52% 57.52%
=======================================
Files 139 139
Lines 13339 13339
=======================================
Hits 7673 7673
Misses 4484 4484
Partials 1182 1182 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
openshift-ci
bot
added
lgtm
perdasilva
removed
the
approved
|
I have some reservations because this is an update to a new version of bingo as well as a new version of golangci-lint. Also, when we have lint issues here, we typically:
I'll need another pass for 3 above. Until then, please |
openshift-ci
bot
added
the
do-not-merge/hold
|
Hi @grokspawn Thank you for looking this one. Regards: #1898 (comment) We solved all suggestions. |
Can we start by splitting this PR into two?
I have some concerns that the bingo updates might have impacts during vendor repackaging, so I'd prefer to see it handled disjointly. What I was saying before is that the SQLite-related code is EOL and we generally do not touch it. I haven't made another pass to see if any proposed changes here are in that area, but I'll be happy to review the new PRs! |
|
@camilamacedo86 can we split this up as suggested by @grokspawn. It's been sitting here for a month, |
openshift-ci
bot
added
approved
|
@grokspawn it is updated as requested. |
| // along the way, uses a highwaterChannel marker to identify the "most stable" channel head to be used as the default channel for the generated package | ||
| func (sv *SemverTemplateData) generateChannels(semverChannels *bundleVersions) []declcfg.Channel { | ||
| outChannels := []declcfg.Channel{} | ||
| outChannels := make([]declcfg.Channel, 0, len(*semverChannels)*2) |
There was a problem hiding this comment.
This is the only change where I have doubts. Because the correct number of entries here can only be identified by examining the input and determining the number of major- and minor-version bumps present in the edge-set... this is likely an arbitrary figure and not especially accurate.
However, since this is in the semver template code which makes no guarantees about resource use or processing duration, it's not wrong.
There was a problem hiding this comment.
But it will reduce the number of reallocations.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: grokspawn, perdasilva The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
openshift-merge-bot
bot
merged commit d47604f
into
operator-framework:master
4 participants
No description provided.