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

Enable Golangci-lint #224

Merged
merged 9 commits into from
Jun 20, 2024
Merged

Enable Golangci-lint #224

merged 9 commits into from
Jun 20, 2024

Conversation

2opremio
Copy link
Contributor

@2opremio 2opremio commented Jun 18, 2024

What

Enable GolangCI-lint in Pull requests

I tried to disable linters I found unreasonable, but I may have forgotten some of them.

I fixed a lot of issues (many of them legitimate). But after 4 hours I got sick of it, so people will need to fix more issues as they change code.

GolangCI-lint, until we clean everything up, is set to bark on code you change.

I may spend another long session cleaning up stuff, but not for today.

Why

It wasn't enforced until now.

Known limitations

You will have to be patient with false positives and fixing code unrelated to your PR until it stabilizes.

It wasn't enforced until now.

I tried to disable linters I found unreasonable, but I may have left some of them.

I fixed a lot of issues (many of them legitimate). But after 4 hours I got sick of it,
so people will need to fix more issues as they change code (golangci, for now at least,
is set to bark of code you change).

I may spend another long session cleaning up stuff, but not for today.
@2opremio 2opremio changed the title Enable Golangci Enable Golangci-lint Jun 18, 2024
@2opremio 2opremio force-pushed the enable-golangci branch 2 times, most recently from 3f3cacc to 19b5c3e Compare June 18, 2024 23:33
Copy link
Contributor

@psheth9 psheth9 left a comment

Choose a reason for hiding this comment

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

just few comments. Lots of small non violent linter fixes, overall looks good to me.

Awesome work !!

.github/workflows/golang.yml Show resolved Hide resolved
.golangci.yml Show resolved Hide resolved
@2opremio 2opremio merged commit 6f92bbf into stellar:main Jun 20, 2024
37 checks passed
@2opremio 2opremio deleted the enable-golangci branch June 20, 2024 11:18
2opremio added a commit to 2opremio/soroban-rpc that referenced this pull request Jul 17, 2024
I broke it at stellar#228

It turns out that `github.com/stellar/go/support/errors.Wrapf` returns nil if the error is nil
(which is different to how `errors.Errorf` behaves).

I have looked at both stellar#228
and stellar#224 and this is the
only instance in which we replaced `Wrapf` by `Errorf` where we
don't first check the error for `nil`.
2opremio added a commit to 2opremio/soroban-rpc that referenced this pull request Jul 17, 2024
I broke it at stellar#228

It turns out that `github.com/stellar/go/support/errors.Wrapf` returns nil if the error is nil
(which is different to how `errors.Errorf` behaves).

I have looked at both stellar#228
and stellar#224 and this is the
only instance in which we replaced `Wrapf` by `Errorf` where we
don't first check the error for `nil`.
2opremio added a commit that referenced this pull request Jul 17, 2024
I broke it at #228

It turns out that `github.com/stellar/go/support/errors.Wrapf` returns nil if the error is nil
(which is different to how `errors.Errorf` behaves).

I have looked at both #228
and #224 and this is the
only instance in which we replaced `Wrapf` by `Errorf` where we
don't first check the error for `nil`.
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