Enable testifylint and tenv linters in golangci-lint #13824

New Issue

2025-11-02T10:54:11-06:00

GiteaMirror commented

2025-11-02 10:54:11 -06:00

Originally created by @TheFox0x7 on GitHub (Dec 14, 2024).

Feature Description

I have contributed this change to Forgejo and I would like to upstream it to Gitea.

Let me know what's the best/recommended way to do this as it did end up large and not pleasant to look through.

I have a few more things that I want to upstream, but I'll have to see first how the codebases differ - maybe they aren't needed.

Screenshots

No response

Originally created by @TheFox0x7 on GitHub (Dec 14, 2024). ### Feature Description [I have contributed this change to Forgejo](https://codeberg.org/forgejo/forgejo/pulls/4535) and I would like to upstream it to Gitea. Let me know what's the best/recommended way to do this as it did end up large and not pleasant to look through. I have a few more things that I want to upstream, but I'll have to see first how the codebases differ - maybe they aren't needed. ### Screenshots _No response_

GiteaMirror added the type/proposal label 2025-11-02 10:54:11 -06:00

GiteaMirror closed this issue

2025-11-02 10:54:11 -06:00

GiteaMirror commented

2025-11-02 10:54:12 -06:00

@silverwind commented on GitHub (Dec 14, 2024):

One question: Why remove skip-dirs? As far as I'm aware, it may increase linting performance by not having golangci-lint crawl into node_modules etc. I'm not totally sure it works because it may need https://github.com/golang/go/issues/42965, but I figured at least it can not hurt.

Edit: It seems we already removed skip-dirs in gitea, so might be okay. I don't recall the detail why it was done.

@silverwind commented on GitHub (Dec 14, 2024): One question: Why remove `skip-dirs`? As far as I'm aware, it may increase linting performance by not having golangci-lint crawl into `node_modules` etc. I'm not totally sure it works because it may need https://github.com/golang/go/issues/42965, but I figured at least it can not hurt. Edit: It seems we already removed `skip-dirs` in gitea, so might be okay. I don't recall the detail why it was done.

GiteaMirror commented

2025-11-02 10:54:12 -06:00

@silverwind commented on GitHub (Dec 14, 2024):

Also, I'm not sure whether the massive refactor for https://github.com/Antonboom/testifylint#require-error is really justified. It may be more useful being able to see "all errors" instead of just the first.

@silverwind commented on GitHub (Dec 14, 2024): Also, I'm not sure whether the massive refactor for https://github.com/Antonboom/testifylint#require-error is really justified. It may be more useful being able to see "all errors" instead of just the first.

GiteaMirror commented

2025-11-02 10:54:12 -06:00

@TheFox0x7 commented on GitHub (Dec 14, 2024):

I think it just was dead config. golangci-lint v1.57 changed run.skip-xxx to issues.exclude-xxx. You moved it in https://github.com/go-gitea/gitea/commit/e6d3f9fc07d193ce95cf0964f0d12da87156fac9

As for the require error, I do see your point, and it does make the change a lot smaller (3183 issues are from that). Though it does make sense in places there it's just if !assert.NoError(t, err) { return } as it serves basically the same purpose as far as I can tell. Another case being test setup, where if it failed, it doesn't make much sense to run the test in the first place.

I can not port that part for now which would make the PR a lot smaller and port require to setups/returns in smaller chunks when adding tests etc. That would make it more manageable and easier to backport if needed.

@TheFox0x7 commented on GitHub (Dec 14, 2024): I think it just was dead config. golangci-lint v1.57 changed `run.skip-xxx` to `issues.exclude-xxx`. You moved it in https://github.com/go-gitea/gitea/commit/e6d3f9fc07d193ce95cf0964f0d12da87156fac9 As for the require error, I do see your point, and it does make the change a lot smaller (3183 issues are from that). Though it does make sense in places there it's just `if !assert.NoError(t, err) { return }` as it serves basically the same purpose as far as I can tell. Another case being test setup, where if it failed, it doesn't make much sense to run the test in the first place. I can not port that part for now which would make the PR a lot smaller and port require to setups/returns in smaller chunks when adding tests etc. That would make it more manageable and easier to backport if needed.

GiteaMirror commented

2025-11-02 10:54:13 -06:00

@silverwind commented on GitHub (Dec 15, 2024):

I think it just was dead config. golangci-lint v1.57 changed run.skip-xxx to issues.exclude-xxx. You moved it in https://github.com/go-gitea/gitea/commit/e6d3f9fc07d193ce95cf0964f0d12da87156fac9

Ah yes, we migrated to issues.exclude-dirs already in https://github.com/go-gitea/gitea/commit/e6d3f9fc07d193ce95cf0964f0d12da87156fac9.

I can not port that part for now

Yeah I would suggest disabling require-error in your port, it's too much churn for questionable benefit. I guess with that rule disabled, the diff will be pretty managable in size, right?

@silverwind commented on GitHub (Dec 15, 2024): > I think it just was dead config. golangci-lint v1.57 changed run.skip-xxx to issues.exclude-xxx. You moved it in https://github.com/go-gitea/gitea/commit/e6d3f9fc07d193ce95cf0964f0d12da87156fac9 Ah yes, we migrated to `issues.exclude-dirs` already in https://github.com/go-gitea/gitea/commit/e6d3f9fc07d193ce95cf0964f0d12da87156fac9. > I can not port that part for now Yeah I would suggest disabling `require-error` in your port, it's too much churn for questionable benefit. I guess with that rule disabled, the diff will be pretty managable in size, right?

GiteaMirror commented

2025-11-02 10:54:13 -06:00

@TheFox0x7 commented on GitHub (Dec 15, 2024):

Yeah it's a lot smaller without it. I'll submit it then.

@TheFox0x7 commented on GitHub (Dec 15, 2024): Yeah it's a lot smaller without it. I'll submit it then.

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/gitea#13824