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.

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

golangci-lint: skip prealloc/fieldalignment in all test files #7263

Merged
merged 2 commits into from
Jan 21, 2025

Conversation

torcolvin
Copy link
Collaborator

There are test files named *test*.go like utilities_testing.go or hlv_utilities_testing.go and we do not care about aligning struct fields or preallocating maps/arrays for these.

.golangci.yml Outdated
@@ -70,10 +70,12 @@ linters-settings:
# Disable goconst in test files, often we have duplicated strings across tests, but don't make sense as constants.
issues:
exclude-rules:
- path: (_test\.go|utilities_testing\.go)
# cover utilities_testing.go and _test.go files
- path: (_test.*go)
Copy link
Member

Choose a reason for hiding this comment

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

I think the .go suffix still needs escaping with \.go

Suggested change
- path: (_test.*go)
- path: (_test.*\.go)

Copy link
Member

@bbrks bbrks Jan 14, 2025

Choose a reason for hiding this comment

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

These previous suggestions/fixes have been lost in the force push

.golangci.yml Outdated Show resolved Hide resolved
@torcolvin torcolvin assigned bbrks and unassigned torcolvin Jan 10, 2025
bbrks
bbrks previously approved these changes Jan 10, 2025
.golangci.yml Outdated
@@ -70,10 +70,12 @@ linters-settings:
# Disable goconst in test files, often we have duplicated strings across tests, but don't make sense as constants.
issues:
exclude-rules:
- path: (_test\.go|utilities_testing\.go)
# cover utilities_testing.go and _test.go files
- path: (_test.*go)
Copy link
Member

@bbrks bbrks Jan 14, 2025

Choose a reason for hiding this comment

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

These previous suggestions/fixes have been lost in the force push

@bbrks bbrks assigned torcolvin and unassigned bbrks Jan 14, 2025
@torcolvin torcolvin force-pushed the ci-golangci-test-skip branch from 0b77c01 to b7a39d2 Compare January 14, 2025 14:46
Copy link
Collaborator Author

@torcolvin torcolvin left a comment

Choose a reason for hiding this comment

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

I did another force push and I updated the rules to account for base/util_testing.go

@torcolvin torcolvin assigned bbrks and unassigned torcolvin Jan 14, 2025
@bbrks
Copy link
Member

bbrks commented Jan 14, 2025

I did another force push and I updated the rules to account for base/util_testing.go

Maybe I just don't understand this syntax...

Wouldn't util_testing.go already be covered by the .* part of the _test.*\.go rule, since ing is a suffix of _test preceeding .go?

@torcolvin
Copy link
Collaborator Author

I did another force push and I updated the rules to account for base/util_testing.go

Maybe I just don't understand this syntax...

Wouldn't util_testing.go already be covered by the .* part of the _test.*\.go rule, since ing is a suffix of _test preceeding .go?

user error, stand by

@torcolvin torcolvin requested a review from bbrks January 20, 2025 15:25
@bbrks bbrks merged commit 14db375 into main Jan 21, 2025
38 checks passed
@bbrks bbrks deleted the ci-golangci-test-skip branch January 21, 2025 15:25
torcolvin added a commit that referenced this pull request Jan 21, 2025
* Cover all test files

* Mark only gosimple as a disabled linter, future proof removing disabled linters in the future, fix regex again
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