-
Notifications
You must be signed in to change notification settings - Fork 939
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
chore: remove dead code #5441
chore: remove dead code #5441
Conversation
Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com>
Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com>
Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com>
@@ -202,12 +202,6 @@ func (e *Environment) getSuiteInstallOpts(suite string, extra ...helm.Option) [] | |||
return append(opts, extra...) | |||
} | |||
|
|||
// GetSelectedSuiteInstallOpts returns the helm install options for the | |||
// selected suite, appending additional specified ones. | |||
func (e *Environment) GetSelectedSuiteInstallOpts(extra ...helm.Option) []helm.Option { |
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.
i skimmed through the blog post, but it wasn't clear to me how the tool can know there are no consumers of exported functions. could they be in use in other repos?
i had the same question the exported functions from other files in this PR, but realized they live in the internal
package, which can't be imported from outside this repo.
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.
no, sure it doesn't check that, I did a quick scan through GitHub, the only things I spotted importing github.com/crossplane/crossplane/test/e2e/...
were forks of Crossplane, but I wasn't extremely thorough in my search as I thought it would be safe to assume no one would import Crossplane just for the test functions 😅 but you are right, it's still part of our public API and I might have been a bit too aggressive 😅 the rest of the stuff outside test/e2e
is in internal
packages, so it's safe to delete.
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.
ah okay thanks for the explanation @phisco - are you planning to re-include the test/e2e
functions? your comment seemed like you might do that, but I didn't see another push here :)
Can we make golangci-lint detect these automatically? We currently have golangci/golangci-lint#1841 says that |
Crossplane does not currently have enough maintainers to address every issue and pull request. This pull request has been automatically marked as |
Description of your changes
Just some cleanup of unused functions detected using deadcode.
I have:
make reviewable
to ensure this PR is ready for review.Added or updated unit tests.Added or updated e2e tests.Linked a PR or a docs tracking issue to document this change.Addedbackport release-x.y
labels to auto-backport this PR.Need help with this checklist? See the cheat sheet.