Skip to content

Conversation

thanethomson
Copy link
Contributor

@thanethomson thanethomson commented Sep 14, 2023

Follows from the govulncheck failure in #1342, since Go v1.19 has reached EOL.


PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
@thanethomson thanethomson requested a review from a team as a code owner September 14, 2023 21:18
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
@thanethomson
Copy link
Contributor Author

In order to get the linter to actually work on v0.34.x, I had to update the v0.34.x CI and code to use the latest golangci-lint, update the linter config, and add some ignores for specific lints in certain places.

If we're going to maintain the v0.34.x branch for much longer, we should possibly consider re-enabling some of the linters I've disabled and fix the lints (a trivial, but time-consuming and low-impact job).

Comment on lines 711 to 715
SendQueueSize: int(atomic.LoadInt32(&channel.sendQueueSize)),
SendQueueSize: int(atomic.LoadInt32(&channel.sendQueueSize)), //nolint:gosec
Priority: channel.desc.Priority,
RecentlySent: atomic.LoadInt64(&channel.recentlySent),
RecentlySent: atomic.LoadInt64(&channel.recentlySent), //nolint:gosec
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to manually skip the gosec linter checks here, otherwise it complains:

G601: Implicit memory aliasing in for loop.

My understanding of how this code works is that we're immediately loading the int values from the pointers during the current iteration, so it's irrelevant. Please correct me if I'm wrong though.

Copy link
Contributor

@sergio-mena sergio-mena Sep 26, 2023

Choose a reason for hiding this comment

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

I think it is complaining about the loop variable (potentially) being rewritten while using it as a reference. IICR @jmalicevic fixed this in other branches

@sergio-mena sergio-mena merged commit 54d2a76 into v0.34.x Sep 26, 2023
@sergio-mena sergio-mena deleted the thane/v0.34.x/bump-go-1.20 branch September 26, 2023 09:50
@lasarojc lasarojc mentioned this pull request Nov 15, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants