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

Implementation of a linter to list range loops over a map #551

Merged
merged 2 commits into from
Aug 28, 2024
Merged

Conversation

fernandofcampos
Copy link
Contributor

Purpose of Changes and their Description

Implementing a linter to list range loops over a map, so we can identify those cases more easily.

Link(s) to Ticket(s) or Issue(s) resolved by this PR

https://linear.app/alloralabs/issue/PROTO-2144/linter-to-throw-if-iterating-over-a-map

Are these changes tested and documented?

Yes. New tests were added to validate the linter. The new linter is not enabled on golangci.yml, so it won't interfere with the current lint checks.

Still Left Todo

Fill this out if this is a Draft PR so others can help.

Copy link

github-actions bot commented Aug 27, 2024

The latest Buf updates on your PR. Results from workflow Buf Linter / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed⏩ skippedAug 27, 2024, 11:20 PM

- ".*\\.pb\\.go"
- ".*\\.pb\\.gw\\.go"
- ".*\\.pulsar\\.go"
- ".*_mocks\\.go"
Copy link
Collaborator

Choose a reason for hiding this comment

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

should you exclude the linter dir itself so you can keep your test?

Copy link
Contributor Author

@fernandofcampos fernandofcampos Aug 27, 2024

Choose a reason for hiding this comment

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

It is not reporting anything on my tests, which is strange. Investigating why, golangci automatically excludes these dirs:
vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
So we don't need to add the linter dir.

@kpeluso kpeluso merged commit abfd120 into dev Aug 28, 2024
7 checks passed
@kpeluso kpeluso deleted the PROTO-2144 branch August 28, 2024 04:56
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