Fix usage of "Deprecated" godoc comments

[alexbozhenko]

There couple of places where Deprecated was not on a separate paragraph. Fix it.

From https://go.dev/blog/godoc

To signal that an identifier should not be used, add a paragraph to its doc comment that begins with “Deprecated:” followed by some information about the deprecation.

Also:
https://github.com/golang/pkgsite/blob/master/internal/godoc/dochtml/deprecated_test.go#L14

Even though in pkgsite Deprecated badge is added only for functions, it still makes sense to have it on a separate paragraph for variables, etc..

E.g. a deprecated Variable from here:
https://pkg.go.dev/net/http#pkg-variables
Shows up in strike-through font in vscode:

There probably should be a linter for this, but I have not found one...

This shows all the deprecations are on separate paragraph now:

# rg -i -n --color always -B2 -A1 '//.*deprecated' | less -S -R

[alexbozhenko]

@piotrpio Now linting fails with the following. How do you recommend dealing with it?

# GOFLAGS="-mod=mod -modfile=go_test.mod" staticcheck ./...
jetstream/test/kv_test.go:907:11: kv.Keys is deprecated: Use ListKeys instead to avoid memory issues.  (SA1019)
...

Just add a bunch of lint:ignores? https://staticcheck.dev/docs/configuration/#line-based-linter-directives

[piotrpio]

I think so, since these are tests for this deprecated method (and we're not using it outside of those tests), so lint:ignore should be a correct approach.

[alexbozhenko]

@piotrpio Thanks! Ready for review.

[alexbozhenko]

eh, now golangci-lint picks it up...
golangci-lint run --timeout 5m0s ./jetstream/...

And that requires a different directive:
https://golangci-lint.run/usage/false-positives/#nolint-directive
https://github.com/golangci/golangci-lint/tree/master/pkg/golinters/nolintlint/internal

Should we get rid of separate statickcheck run, and let golangci-lint handle all the linting?
https://github.com/alexbozhenko/nats.go/blob/c9acd5a0af476bd5ecf70f83f58fed7a73a32ee5/.github/workflows/ci.yaml#L35

[piotrpio]

I'm afraid we'll have many more linting errors in nats package if we enable golangci-lint, unless we disable most of the linters and only enable staticcheck. When working on jetstream package I used golangci-lint as this was a new package and I went for a bit more aggressive linting than we had in the past. For now I would probably go with nolint directive and consider switching to golangci-lint in nats package in a different PR (although I am not sold on the idea yet).

[ripienaar]

We've also agreed to keep our linting close to reasonable as just what go standard formatting would require + static check + vet.

So enabling the additional lints will just cause a bunch of needless code cancer for no benefit.

[coveralls]

coverage: 84.904% (-0.05%) from 84.949%
when pulling 5baa151 on alexbozhenko:fix_deprecated
into 1e1519f on nats-io:main.

[ripienaar]

I would strongly vote to remove golang-ci from here, what makes go great is that everyone agrees on a basic styles and validations - adding golang-ci here is a hurdle everyone has to jump and as we see we enter a cycle of adding code for no purpose than to make the linters happy.

We had calls around this a few months ago and agreed to remove additional tests over vet and staticcheck

[alexbozhenko]

So, because staticcheck command runs on the entire repo, and then golangci-lint config includes staticcheck, we would have to add two ignore directives, as I showed here:
5baa151#diff-0bcc030754268fec1fe3e8f34d019b28a6ff8cb6f8a85e4890f42776f35c5346R907-R908

What I meant is not to change the linter config, but change how the linting is run(#1774), so e.g. deprecated code would need only a single directive for golangci:

	 //nolint:staticcheck // SA1004 testing deprecated api
	_, err = kv.Keys(ctx)

So please advice what to do in this PR. Should we just add two directives, lint:ignore and nolint for now, to land this PR?