-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix go.mod to fix CodeQL workflows #5364
Conversation
Signed-off-by: Jonah Kowall <jkowall@kowall.net>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5364 +/- ##
==========================================
- Coverage 95.21% 95.20% -0.02%
==========================================
Files 343 343
Lines 16777 16777
==========================================
- Hits 15974 15972 -2
- Misses 605 606 +1
- Partials 198 199 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -1,6 +1,6 @@ | |||
module github.com/jaegertracing/jaeger | |||
|
|||
go 1.21 | |||
go 1.21.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
according to LLM specifying patch version in this directive is not recommended. The directive controls source code compatibility, which is not affected by the patch version.
On that topic, most of our CI workflows are using go-version: 1.22.x
. It's fine for test-only workflows, but others are used to build final artifacts, and you could argue not fixing a precise patch version means the builds are not repeatable. That sounds like the issue the security scanner should be flagging, not the go.mod directive
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference, all official examples of go
directive use major.minor version -- https://go.dev/ref/mod#go-mod-file-go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be conflicting, as you can see from CodeQL we are getting a warning for not using the 1.N**.P** part of the toolchain in the file. I guess we can ignore the warning.
Correct the issue in codeQL
Checklist