Repository: go-logr/logr (go)

Issue:
lint: comment check
We were wondering recently why a mismatch between comment and struct member (LogTimestamps vs LogTimestamp) was not reported.

The reason is that golangci-lint by default suppresses several issues, including the one about comments. This can be disabled, which then leads to:
```
golangci-lint run -E revive --exclude-use-default=false
funcr/example/main.go:26:6: exported: exported type E should have comment or be unexported (revive)
type E struct {
     ^
funcr/example/main.go:34:1: exported: exported function Helper should have comment or be unexported (revive)
func Helper(log logr.Logger, msg string) {
^
logr.go:327:1: exported: comment on exported method Logger.Helper should be of the form "Helper ..." (revive)
// WithCallStackHelper returns a new Logger instance that skips the
^
funcr/funcr.go:83:2: exported: exported const None should have comment (or a comment on this block) or be unexported (revive)
	None MessageClass = iota
	^
funcr/funcr.go:330:1: exported: comment on exported method Formatter.Init should be of the form "Init ..." (revive)
// Note that this receiver is a pointer, so depth can be saved.
^
funcr/funcr.go:335:1: exported: exported method Formatter.Enabled should have comment or be unexported (revive)
func (f Formatter) Enabled(level int) bool {
^
funcr/funcr.go:355:1: exported: comment on exported method Formatter.FormatError should be of the form "FormatError ..." (revive)
// FormatInfo flattens an Error log message into strings.
^
funcr/funcr.go:386:1: exported: exported method Formatter.AddValues should have comment or be unexported (revive)
func (f *Formatter) AddValues(kvList []interface{}) {
^
funcr/funcr.go:392:1: exported: exported method Formatter.AddCallDepth should have comment or be unexported (revive)
func (f *Formatter) AddCallDepth(depth int) {
```

I don't mind being stricter, i.e. adding `--exclude-use-default=false` and fixing these findings.

Pull request:
use golangci-lint-action
The integration with GitHub is better. It uses caching and posts
errors as annotations to the PR. This is easier to use than having to
dig into the log to find errors.

Fixes #82

Patch:
diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml
index c366cfc..d76ae31 100644
--- a/.github/workflows/lint.yaml
+++ b/.github/workflows/lint.yaml
@@ -6,32 +6,37 @@ jobs:
   lint:
     runs-on: ubuntu-latest
     steps:
-      - name: Install Go
-        uses: actions/setup-go@v2
       - name: Checkout code
         uses: actions/checkout@v2
       - name: Lint
-        run: |
-          docker run --rm -v `pwd`:/go/src/prj -w /go/src/prj \
-            golangci/golangci-lint golangci-lint run \
-                -v \
-                --max-same-issues 10 \
-                --disable-all \
-                -E asciicheck \
-                -E deadcode \
-                -E errcheck \
-                -E forcetypeassert \
-                -E gocritic \
-                -E gofmt \
-                -E goimports \
-                -E gosimple \
-                -E govet \
-                -E ineffassign \
-                -E misspell \
-                -E revive \
-                -E staticcheck \
-                -E structcheck \
-                -E typecheck \
-                -E unused \
-                -E varcheck
+        uses: golangci/golangci-lint-action@v2
+        with:
+          # version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
+          version: latest
+
+          # Optional: show only new issues if it's a pull request. The default value is `false`.
+          # only-new-issues: true
 
+          # golangci-lint command line arguments.
+          args:
+                -v
+                --max-same-issues 10
+                --disable-all
+                --exclude-use-default=false
+                -E asciicheck
+                -E deadcode
+                -E errcheck
+                -E forcetypeassert
+                -E gocritic
+                -E gofmt
+                -E goimports
+                -E gosimple
+                -E govet
+                -E ineffassign
+                -E misspell
+                -E revive
+                -E staticcheck
+                -E structcheck
+                -E typecheck
+                -E unused
+                -E varcheck
diff --git a/funcr/example/main.go b/funcr/example/main.go
index 0a8131d..303707b 100644
--- a/funcr/example/main.go
+++ b/funcr/example/main.go
@@ -23,15 +23,15 @@ import (
 	"github.com/go-logr/logr/funcr"
 )
 
-type E struct {
+type e struct {
 	str string
 }
 
-func (e E) Error() string {
+func (e e) Error() string {
 	return e.str
 }
 
-func Helper(log logr.Logger, msg string) {
+func helper(log logr.Logger, msg string) {
 	helper2(log, msg)
 }
 
@@ -54,6 +54,6 @@ func example(log logr.Logger) {
 	log.V(1).Info("you should see this")
 	log.V(1).V(1).Info("you should NOT see this")
 	log.Error(nil, "uh oh", "trouble", true, "reasons", []float64{0.1, 0.11, 3.14})
-	log.Error(E{"an error occurred"}, "goodbye", "code", -1)
-	Helper(log, "thru a helper")
+	log.Error(e{"an error occurred"}, "goodbye", "code", -1)
+	helper(log, "thru a helper")
 }
diff --git a/funcr/funcr.go b/funcr/funcr.go
index ad9e73e..107493b 100644
--- a/funcr/funcr.go
+++ b/funcr/funcr.go
@@ -80,9 +80,13 @@ type Options struct {
 type MessageClass int
 
 const (
+	// None ignores all message classes.
 	None MessageClass = iota
+	// All considers all message classes.
 	All
+	// Info only considers info messages.
 	Info
+	// Error only considers error messages.
 	Error
 )
 
@@ -327,11 +331,12 @@ func (f Formatter) caller() callerID {
 	return callerID{filepath.Base(file), line}
 }
 
-// Note that this receiver is a pointer, so depth can be saved.
+// Init uses a pointer receiver, so depth can be saved.
 func (f *Formatter) Init(info logr.RuntimeInfo) {
 	f.depth += info.CallDepth
 }
 
+// Enabled checks whether an info message at the given level should be logged.
 func (f Formatter) Enabled(level int) bool {
 	return level <= f.verbosity
 }
@@ -352,7 +357,7 @@ func (f Formatter) FormatInfo(level int, msg string, kvList []interface{}) (pref
 	return f.prefix, flatten(args...)
 }
 
-// FormatInfo flattens an Error log message into strings.
+// FormatError flattens an Error log message into strings.
 // The prefix will be empty when no names were set.
 func (f Formatter) FormatError(err error, msg string, kvList []interface{}) (prefix, argsStr string) {
 	args := make([]interface{}, 0, 64) // using a constant here impacts perf
@@ -383,12 +388,15 @@ func (f *Formatter) AddName(name string) {
 	f.prefix += name
 }
 
+// AddValues appends the specified values.
 func (f *Formatter) AddValues(kvList []interface{}) {
 	// Three slice args forces a copy.
 	n := len(f.values)
 	f.values = append(f.values[:n:n], kvList...)
 }
 
+// AddCallDepth increases the number of stack frames that need to be
+// skipped.
 func (f *Formatter) AddCallDepth(depth int) {
 	f.depth += depth
 }
diff --git a/logr.go b/logr.go
index 26dc17d..d3ae35b 100644
--- a/logr.go
+++ b/logr.go
@@ -21,7 +21,8 @@ limitations under the License.
 // to back that API.  Packages in the Go ecosystem can depend on this package,
 // while callers can implement logging with whatever backend is appropriate.
 //
-// # Usage
+// Usage
+// -----
 //
 // Logging is done using a Logger instance.  Logger is a concrete type with
 // methods, which defers the actual logging to a LogSink interface.  The main
@@ -45,7 +46,8 @@ limitations under the License.
 // LogSink implementations can choose to do things like attach additional
 // information (such as stack traces) on calls to Error().
 //
-// # Verbosity
+// Verbosity
+// ---------
 //
 // Often we want to log information only when the application in "verbose
 // mode".  To write log-lines that are more verbose, Logger has a V() method.
@@ -62,7 +64,8 @@ limitations under the License.
 // We can write:
 //   logger.V(2).Info("an unusual thing happened")
 //
-// # Logger Names
+// Logger Names
+// ------------
 //
 // Logger instances can have name strings so that all messages logged through
 // that instance have additional context.  For example, you might want to add
@@ -79,7 +82,8 @@ limitations under the License.
 // joining operation (e.g. whitespace, commas, periods, slashes, brackets,
 // quotes, etc).
 //
-// # Saved Values
+// Saved Values
+// ------------
 //
 // Logger instances can store any number of key/value pairs, which will be
 // logged alongside all messages logged through that instance.  For example,
@@ -97,7 +101,8 @@ limitations under the License.
 //   // later on...
 //   obj.logger.Info("setting foo", "value", targetValue)
 //
-// # Best Practices
+// Best Practices
+// --------------
 //
 // Logger has very few hard rules, with the goal that LogSink implementations
 // might have a lot of freedom to differentiate.  There are, however, some
@@ -112,7 +117,8 @@ limitations under the License.
 // may be any Go value, but how the value is formatted is determined by the
 // LogSink implementation.
 //
-// # Key Naming Conventions
+// Key Naming Conventions
+// ----------------------
 //
 // Keys are not strictly required to conform to any specification or regex, but
 // it is recommended that they:
@@ -130,21 +136,22 @@ limitations under the License.
 // generally best to avoid using the following keys, as they're frequently used
 // by implementations:
 //
-//   * "caller": the calling information (file/line) of a particular log line.
-//   * "error": the underlying error value in the `Error` method.
-//   * "level": the log level.
-//   * "logger": the name of the associated logger.
-//   * "msg": the log message.
-//   * "stacktrace": the stack trace associated with a particular log line or
-//                   error (often from the `Error` message).
-//   * "ts": the timestamp for a log line.
+//   * `"caller"`: the calling information (file/line) of a particular log line.
+//   * `"error"`: the underlying error value in the `Error` method.
+//   * `"level"`: the log level.
+//   * `"logger"`: the name of the associated logger.
+//   * `"msg"`: the log message.
+//   * `"stacktrace"`: the stack trace associated with a particular log line or
+//                     error (often from the `Error` message).
+//   * `"ts"`: the timestamp for a log line.
 //
 // Implementations are encouraged to make use of these keys to represent the
 // above concepts, when necessary (for example, in a pure-JSON output form, it
 // would be necessary to represent at least message and timestamp as ordinary
 // named values).
 //
-// # Break Glass
+// Break Glass
+// -----------
 //
 // Implementations may choose to give callers access to the underlying
 // logging implementation.  The recommended pattern for this is:
@@ -332,7 +339,7 @@ func (l Logger) WithCallDepth(depth int) Logger {
 // it supports a WithCallStackHelper() method, that will be also
 // called. If the implementation does not support either of these, the
 // original Logger will be returned.
-func (l Logger) Helper() (func(), Logger) {
+func (l Logger) WithCallStackHelper() (func(), Logger) {
 	var helper func()
 	if withCallDepth, ok := l.sink.(CallDepthLogSink); ok {
 		l.setSink(withCallDepth.WithCallDepth(1))


Test patch:
diff --git a/testing/test_test.go b/testing/test_test.go
index 61a1315..1c558d9 100644
--- a/testing/test_test.go
+++ b/testing/test_test.go
@@ -40,13 +40,13 @@ func TestLogger(t *testing.T) {
 }
 
 func Helper(log logr.Logger, msg string) {
-	helper, log := log.Helper()
+	helper, log := log.WithCallStackHelper()
 	helper()
 	helper2(log, msg)
 }
 
 func helper2(log logr.Logger, msg string) {
-	helper, log := log.Helper()
+	helper, log := log.WithCallStackHelper()
 	helper()
 	log.Info(msg)
 }