Merged
Conversation
e4a5451 to
7eacc77
Compare
7eacc77 to
9e3f912
Compare
antgamdia
reviewed
Feb 17, 2026
antgamdia
reviewed
Feb 17, 2026
Comment on lines
53
to
56
| - name: deadcode | ||
| run: | | ||
| go install golang.org/x/tools/cmd/deadcode@latest | ||
| make deadcode |
There was a problem hiding this comment.
comment : this way, we don't know which version of the linter we are using. While we might want to stick to the latest, it can often make our CI fail. Instead, we can define the deps in a non-compiled file, track them in our deps and install them via go install, something like:
//go:build tools
package tools
import (
_ "golang.org/x/tools/cmd/deadcode"
)dev-deps:
go install \
golang.org/x/tools/cmd/deadcode
arbulu89
commented
Feb 17, 2026
|
|
||
| require ( | ||
| github.com/chigopher/pathlib v1.0.0 // indirect | ||
| github.com/chigopher/pathlib v0.19.1 // indirect |
Contributor
Author
There was a problem hiding this comment.
this package was downversioned by the author:
go: warning: github.com/chigopher/pathlib@v1.0.0: retracted by module author: Published accidentally
go: to switch to the latest unretracted version, run:
go get github.com/chigopher/pathlib@latest
8210117 to
a4e193c
Compare
gagandeepb
approved these changes
Feb 18, 2026
Contributor
gagandeepb
left a comment
There was a problem hiding this comment.
Thanks for this.This could be evolved to have a deadcode_ignore file containing either files or file-path-patterns to ignore for the purposes of deadcode analysis.
antgamdia
approved these changes
Feb 18, 2026
antgamdia
left a comment
There was a problem hiding this comment.
Thanks for adding the suggestions and for looking into the go tool thing!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Run golang
deadcodeprocess to find dead code paths and remove them.https://pkg.go.dev/golang.org/x/tools/internal/cmd/deadcode
Here the fixed errors:
https://github.com/trento-project/agent/actions/runs/22100883779/job/63869965678?pr=552
3 things:
asdfplugin for golang deadcode, so we need to install and reshim manuallydeadcodealways returns 0 return code (unless compliation errors), so we need to look for stdout to see if there are errors. That's why we have that comparison in the makefile filetestflag. Otherwise we would have a lot of dead codeEDIT:
When @antgamdia suggested to add
deadcodeto thetools.gofile to track the version, I have read that golang tooling usage has envolved quite a bit since 1.24.Now, tools can be declared and tracked without any dummy file to import them, and use them as
go tool {toolname}. This gives as the chance to remove some dependencies fromasdfas well, and have all golang tooling in the same place.If you want to read a bit more: https://blog.howardjohn.info/posts/go-tools-command/