enforce nolint scope

[TheFox0x7]

enable nolintlint scope requirement
add comments to new directives so it's more obvious why they are in place

---

I can also toggle the mandatory comments on if that's something of interest.

[silverwind]

Nice work. Resolve comments above before merge.

[wxiaoguang]

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

[TheFox0x7]

Go ahead, the maintainer edit thing is checked :)

[silverwind]

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.

[TheFox0x7]

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 cover oauth_* as well) or for migrations only.
See text based rules which I think would work for this

[silverwind]

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

[TheFox0x7]

var-naming: don't use an underscore in package name

[silverwind]

I'd vote for disabling var-naming completely. This is the kind of rule I don't want to have because it's stylistic. Focus of linting should be primary in finding bugs, not style issues.

[silverwind]

Alternatively, just disabling package name checks should also work:

linters:
  settings:
    revive:
      rules:
        - name: var-naming
          arguments:
            - skip-package-name-checks: true

[silverwind]

Maybe better overall to disable var-naming completely. @TheFox0x7 would you like to do it in this PR or we merge this and do it in another.

[wxiaoguang]

Made some changes:

• rewrite TestCleanUploadFileName, no "nolint" now
• shorten some comments since we have used "nolint" then "it shouldn't be an issue"
  • "underscore in migration packages isn't a large issue" -> "version underscore"
  • " export stutter, not a large issue" -> " export stutter"