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

Update lint-project.sh #280

Closed
wants to merge 1 commit into from

Conversation

alovak
Copy link

@alovak alovak commented Sep 21, 2023

The lint-project.sh script currently runs golangci-lint in two separate instances. Initially, it's executed with all linters turned off except for forbidigo (see reference). Following this, it is run again, this time with all linters enabled except for forbidigo (see reference).

To bypass the forbidigo linter for specific files that utilize panic, the //nolint directive was used. This directive was effective only during the first instance of the linting process. However, during the second linting round, the //nolint directive becomes superfluous since forbidigo is not active. This redundancy leads to the detection of an unused nolintlint directive error, as illustrated in the following output:

Error: message.go:42:14: directive `//nolint // as specs moslty static, we panic on spec validation errors` is unused (nolintlint)
		panic(err) //nolint // as specs moslty static, we panic on spec validation errors

With this PR, we introduce the GOLANGCI_ALLOW_PANIC environment variable. When it's set to yes, no checks for panic are performed by forbidigo. It's done similarly to GOLANGCI_ALLOW_PRINT.

P.S. it's not a viable option to remove panic from https://github.com/moov-io/iso8583/ (see moov-io/iso8583#269)

alovak added a commit to moov-io/iso8583 that referenced this pull request Sep 22, 2023
@alovak
Copy link
Author

alovak commented Sep 22, 2023

This //nolint:forbidigo,nolintlint helped me. No need in the PR.

@alovak alovak closed this Sep 22, 2023
alovak added a commit to moov-io/iso8583 that referenced this pull request Sep 26, 2023
* draft data race solution for message

* make message thread safe

* remove //nolint as it does not work

moov-io/infra#280

* add test for concurrent access

* protect Composite field against concurrent access

* satisfy linkers

* remove unnecessary comments and address feedback
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.

3 participants