Hmm, adding comments make it harder to read ...

I can help to rewrite the code to something like

for testCase := range cases {    assert.Equal(....)}

(if you don't mind)

Then we can get rid of the "nolint" tricks

Go ahead, the maintainer edit thing is checked :)

Yes, eliminating nolints is better. I think something ought to be done about the migration packages too. Maybe just a config in.golangci.yml that disables revive for those files, but it's a pretty broad disable.

Those are fixable with a rename. I don't suppose that's breaking as migrations are internal and I'd be surprised if something in our code relied on package name. It's more of an issue of what name should there be.
There's also not much benefit in that and the_ rule could be suppressed globally (to coveroauth_* as well) or for migrations only.
See text based rules which I think would work for this

Which specificrevive rule is being triggered by these underscores? Maybe we can disable the rule itself.

So why do you think we can't disablevar-naming exactly?

See:But we can't fully disable it, because it also check other places like ID in models

There are too many unclear parts.

I'm not sure what the ID topic is about, but then let's go with theskip-package-name-checks option?

but then let's go with theskip-package-name-checks option?

Agree

Done. Added comments in80b05b7 to document why those 3 rules are in place.
I'll probably follow this PR up with a comment enforcement - I think the reason for disabling the rule will be very useful long term.

follow up is ready - no clue how to stack stuff here so I'll wait for this to land.

You could go onto this branch andgit cherry-pick <hash> the commit hash(es) from the other branch, but let's do the merge then.