Skip to content

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Mar 13, 2025

This linter checks for unused functions and parameters, and parameters that have only a single value which is why I've introduced a private global variable in the containerd test; that feels a little weird, but I think it's slightly better than having a linter disable comment given it's internal test code.

I've already addressed the main two classes of violations this linter caught in the codebase, so the actual net win for enabling this is the sum of this + these PRs:

There was one new violation that slipped through after #520 was landed, which I've also addressed here

Relates to #274

@G-Rath G-Rath mentioned this pull request Mar 13, 2025
49 tasks
@G-Rath G-Rath force-pushed the linting/enable-unparam branch from f9a5ffb to 6bc973d Compare March 14, 2025 01:21
@another-rex
Copy link
Collaborator

Hmm the containerd one does seem pretty odd, I'm leaning towards adding a nolint there. Reasoning is that it looks like it is a helper function that can support adding additional different test files, just that right now there is only one.

It is much easier to remove the nolint later than it is to add the arguments back. (And we'll have the nolintlint to catch it).

I haven't looked into this too much though, pinging @andreyka who wrote this initially, is my assessment correct?

@G-Rath
Copy link
Collaborator Author

G-Rath commented Mar 14, 2025

Ironically that was originally using a temp dir, but it was hardcoded apparently to fix some issue with running on vms...

I agree it's odd, though I was concerned that nolinting it would disable the lint for the whole function signature rather than just that param

@G-Rath G-Rath force-pushed the linting/enable-unparam branch 6 times, most recently from d4ffc73 to d231165 Compare March 20, 2025 02:49
@G-Rath G-Rath force-pushed the linting/enable-unparam branch from d231165 to bef81ef Compare March 25, 2025 03:17
@G-Rath G-Rath force-pushed the linting/enable-unparam branch from bef81ef to 9f78a42 Compare March 27, 2025 18:55
@copybara-service copybara-service bot merged commit 05c19a8 into google:main Mar 28, 2025
12 checks passed
@G-Rath G-Rath deleted the linting/enable-unparam branch March 28, 2025 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants