Skip to content
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.

Sign up for GitHub

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

Jump to bottom

remove unused variables #99

Merged
merged 3 commits into from
Oct 27, 2023
Merged

remove unused variables #99

merged 3 commits into from
Oct 27, 2023

Conversation

tbauriedel
Copy link
Member

Remove unused variables in tests

@tbauriedel tbauriedel requested a review from martialblog July 24, 2023 13:18
@tbauriedel tbauriedel self-assigned this Jul 24, 2023
@martialblog
Copy link
Member

I will have a look at it this week. Not really sure what these test functions do.

@tbauriedel
Copy link
Member Author

Most (to my knowledge even all) just call their Collect() function without using the t *testing.T variable.

For the future, it would be appropriate to expand the tests somewhat at this point (as far as possible and necessary).

martialblog
martialblog reviewed Jul 24, 2023
View reviewed changes
Copy link
Member

@martialblog martialblog left a comment

Choose a reason for hiding this comment

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

I think we should actually test/assert something here, right now this seems very useless... since it doesn't collect anything if called as such.

If we want to keep it, then maybe we could have a t.Log(c) to at least see what's going on.

@tbauriedel
Copy link
Member Author

We should rather tackle the whole thing directly once cleanly instead of just using the variable somehow.
I'll take care of the whole thing.

@martialblog martialblog force-pushed the fix/remove-unused-variables branch from 4d6c0e2 to 736cb2b Compare October 27, 2023 10:59
tbauriedel and others added 3 commits October 27, 2023 12:59
@tbauriedel @martialblog
Update tests to be more expressive
b9fe671
@martialblog
Update Github Actions
58ff78f 
 - update golangci-lint version
 - update Action versions
 - remove snapshot modus
@martialblog
Fix import style
4d56e9b
@martialblog martialblog force-pushed the fix/remove-unused-variables branch from 736cb2b to 4d56e9b Compare October 27, 2023 11:00
@martialblog
Copy link
Member

I updated the PR.

The Test functions now have more to do and use the testing.T object.

Plus some updates to the CI.

@martialblog martialblog merged commit 5b16bf6 into main Oct 27, 2023
2 checks passed
@martialblog martialblog deleted the fix/remove-unused-variables branch October 27, 2023 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martialblog martialblog martialblog left review comments

Assignees

@martialblog martialblog

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tbauriedel @martialblog