From 2927b7ecf285fd745644ca3f61e3d0d1a5eca0a7 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Fri, 26 Jul 2024 21:18:34 +0900 Subject: [PATCH 1/8] introduce -fail-level flag and deprecate -fail-on-error Now reviewdog can control exit code based on issue severity. For example, if `-fail-level=warning`, then reviewdog will exit with 0 for issues with info severity, while it will exit with 1 if it finds issues with error, warning, or unknown severity. This change also fixes the feature parity with github-[pr-]check reporter. Previously, only this reporter didn't fail on warning or info level issues (#1846), and this behavior was removed during refactoring (#1846). Now users can use `-reporter=github-check -fail-level=error` to restore the previous behavior. --- cmd/reviewdog/main.go | 19 ++++++++-- fail_level.go | 83 +++++++++++++++++++++++++++++++++++++++++++ fail_level_test.go | 73 +++++++++++++++++++++++++++++++++++++ project/run.go | 5 +-- project/run_test.go | 20 +++++------ reviewdog.go | 32 ++++++++--------- reviewdog_test.go | 8 ++--- 7 files changed, 205 insertions(+), 35 deletions(-) create mode 100644 fail_level.go create mode 100644 fail_level_test.go diff --git a/cmd/reviewdog/main.go b/cmd/reviewdog/main.go index c5c463e6..5a466c2e 100644 --- a/cmd/reviewdog/main.go +++ b/cmd/reviewdog/main.go @@ -62,6 +62,7 @@ type option struct { tee bool filterMode filter.Mode failOnError bool + failLevel reviewdog.FailLevel logLevel string } @@ -202,7 +203,8 @@ const ( $ export CI_REPO_OWNER="haya14busa" # repository owner $ export CI_REPO_NAME="reviewdog" # repository name ` - failOnErrorDoc = `Returns 1 as exit code if any errors/warnings found in input` + failOnErrorDoc = `[DEPRECATED] use -fail-level instead` + failLevelDoc = `reviewdog will exit with code 1 if it finds at least 1 issue with severity greater than or equal to the given level. [none(default),any,info,warning,error]` logLevelDoc = `log level for reviewdog itself. (debug, info, warning, error)` ) @@ -225,6 +227,7 @@ func init() { flag.BoolVar(&opt.tee, "tee", false, teeDoc) flag.Var(&opt.filterMode, "filter-mode", filterModeDoc) flag.BoolVar(&opt.failOnError, "fail-on-error", false, failOnErrorDoc) + flag.Var(&opt.failLevel, "fail-level", failLevelDoc) flag.StringVar(&opt.logLevel, "log-level", "info", logLevelDoc) } @@ -465,7 +468,7 @@ func run(r io.Reader, w io.Writer, opt *option) error { } if isProject { - return project.Run(ctx, projectConf, buildRunnersMap(opt.runners), cs, ds, opt.tee, opt.filterMode, opt.failOnError) + return project.Run(ctx, projectConf, buildRunnersMap(opt.runners), cs, ds, opt.tee, opt.filterMode, failLevel(opt)) } p, err := newParserFromOpt(opt) @@ -473,7 +476,7 @@ func run(r io.Reader, w io.Writer, opt *option) error { return err } - app := reviewdog.NewReviewdog(toolName(opt), p, cs, ds, opt.filterMode, opt.failOnError) + app := reviewdog.NewReviewdog(toolName(opt), p, cs, ds, opt.filterMode, failLevel(opt)) return app.Run(ctx, r) } @@ -1029,3 +1032,13 @@ func localDiffService(opt *option) (reviewdog.DiffService, error) { } return diffService(opt.diffCmd, opt.diffStrip) } + +func failLevel(opt *option) reviewdog.FailLevel { + if opt.failOnError { + slog.Warn("reviewdog: -fail-on-error is deprecated. Use -fail-level=any, or -fail-level=error for github-[pr-]check reporter instead.") + if opt.failLevel == reviewdog.FailLevelDefault { + return reviewdog.FailLevelAny + } + } + return opt.failLevel +} diff --git a/fail_level.go b/fail_level.go new file mode 100644 index 00000000..0a1b63db --- /dev/null +++ b/fail_level.go @@ -0,0 +1,83 @@ +package reviewdog + +import ( + "fmt" + + "github.com/reviewdog/reviewdog/proto/rdf" +) + +// FailLevel represents enumeration of available filter modes +type FailLevel int + +const ( + // FailLevelDefault represents default mode, which means users doesn't specify + // fail-level. Basically, it's same as FailLevelNone. + FailLevelDefault FailLevel = iota + FailLevelNone + FailLevelAny + FailLevelInfo + FailLevelWarning + FailLevelError +) + +// String implements the flag.Value interface +func (failLevel *FailLevel) String() string { + names := [...]string{ + "default", + "none", + "any", + "info", + "warning", + "error", + } + if *failLevel < FailLevelDefault || *failLevel > FailLevelError { + return "Unknown failLevel" + } + + return names[*failLevel] +} + +// Set implements the flag.Value interface +func (failLevel *FailLevel) Set(value string) error { + switch value { + case "default", "": + *failLevel = FailLevelDefault + case "none": + *failLevel = FailLevelNone + case "any": + *failLevel = FailLevelAny + case "info": + *failLevel = FailLevelInfo + case "warning": + *failLevel = FailLevelWarning + case "error": + *failLevel = FailLevelError + default: + return fmt.Errorf("invalid failLevel name: %s", value) + } + return nil +} + +// ShouldFail returns true if reviewdog should exit with 1 with given rdf.Severity. +func (failLevel FailLevel) ShouldFail(severity rdf.Severity) bool { + if failLevel == FailLevelDefault || failLevel == FailLevelNone { + return false + } + minSeverity := failLevel.minSeverity() + return minSeverity == rdf.Severity_UNKNOWN_SEVERITY || severity <= minSeverity +} + +func (failLevel FailLevel) minSeverity() rdf.Severity { + switch failLevel { + case FailLevelDefault, FailLevelNone, FailLevelAny: + return rdf.Severity_UNKNOWN_SEVERITY + case FailLevelInfo: + return rdf.Severity_INFO + case FailLevelWarning: + return rdf.Severity_WARNING + case FailLevelError: + return rdf.Severity_ERROR + default: + return rdf.Severity_UNKNOWN_SEVERITY + } +} diff --git a/fail_level_test.go b/fail_level_test.go new file mode 100644 index 00000000..2b5ee85d --- /dev/null +++ b/fail_level_test.go @@ -0,0 +1,73 @@ +package reviewdog + +import ( + "testing" + + "github.com/reviewdog/reviewdog/proto/rdf" +) + +func TestShouldFail(t *testing.T) { + tests := []struct { + failLevel FailLevel + severity rdf.Severity + want bool + }{ + { + failLevel: FailLevelDefault, severity: rdf.Severity_ERROR, want: false, + }, + { + failLevel: FailLevelDefault, severity: rdf.Severity_WARNING, want: false, + }, + { + failLevel: FailLevelDefault, severity: rdf.Severity_INFO, want: false, + }, + { + failLevel: FailLevelDefault, severity: rdf.Severity_UNKNOWN_SEVERITY, want: false, + }, + { + failLevel: FailLevelNone, severity: rdf.Severity_ERROR, want: false, + }, + { + failLevel: FailLevelError, severity: rdf.Severity_ERROR, want: true, + }, + { + failLevel: FailLevelError, severity: rdf.Severity_WARNING, want: false, + }, + { + failLevel: FailLevelError, severity: rdf.Severity_INFO, want: false, + }, + { + failLevel: FailLevelError, severity: rdf.Severity_UNKNOWN_SEVERITY, want: true, + }, + { + failLevel: FailLevelWarning, severity: rdf.Severity_ERROR, want: true, + }, + { + failLevel: FailLevelWarning, severity: rdf.Severity_WARNING, want: true, + }, + { + failLevel: FailLevelWarning, severity: rdf.Severity_INFO, want: false, + }, + { + failLevel: FailLevelWarning, severity: rdf.Severity_UNKNOWN_SEVERITY, want: true, + }, + { + failLevel: FailLevelInfo, severity: rdf.Severity_ERROR, want: true, + }, + { + failLevel: FailLevelInfo, severity: rdf.Severity_WARNING, want: true, + }, + { + failLevel: FailLevelInfo, severity: rdf.Severity_INFO, want: true, + }, + { + failLevel: FailLevelInfo, severity: rdf.Severity_UNKNOWN_SEVERITY, want: true, + }, + } + for _, tt := range tests { + if got := tt.failLevel.ShouldFail(tt.severity); got != tt.want { + t.Errorf("FailLevel(%s).ShouldFail(%s) = %v, want %v", tt.failLevel.String(), + tt.severity, got, tt.want) + } + } +} diff --git a/project/run.go b/project/run.go index d52f03cf..7f21e0f8 100644 --- a/project/run.go +++ b/project/run.go @@ -91,7 +91,8 @@ func RunAndParse(ctx context.Context, conf *Config, runners map[string]bool, def } // Run runs reviewdog tasks based on Config. -func Run(ctx context.Context, conf *Config, runners map[string]bool, c reviewdog.CommentService, d reviewdog.DiffService, teeMode bool, filterMode filter.Mode, failOnError bool) error { +func Run(ctx context.Context, conf *Config, runners map[string]bool, c reviewdog.CommentService, d reviewdog.DiffService, + teeMode bool, filterMode filter.Mode, failLevel reviewdog.FailLevel) error { results, err := RunAndParse(ctx, conf, runners, "", teeMode) // Level is not used. if err != nil { return err @@ -115,7 +116,7 @@ func Run(ctx context.Context, conf *Config, runners map[string]bool, c reviewdog if err := result.CheckUnexpectedFailure(); err != nil { return err } - return reviewdog.RunFromResult(ctx, c, ds, filediffs, d.Strip(), toolname, filterMode, failOnError) + return reviewdog.RunFromResult(ctx, c, ds, filediffs, d.Strip(), toolname, filterMode, failLevel) }) }) return g.Wait() diff --git a/project/run_test.go b/project/run_test.go index f498e70f..f7594385 100644 --- a/project/run_test.go +++ b/project/run_test.go @@ -40,7 +40,7 @@ func TestRun(t *testing.T) { t.Run("empty", func(t *testing.T) { conf := &Config{} - if err := Run(ctx, conf, nil, nil, nil, false, filter.ModeAdded, false); err != nil { + if err := Run(ctx, conf, nil, nil, nil, false, filter.ModeAdded, reviewdog.FailLevelNone); err != nil { t.Error(err) } }) @@ -51,7 +51,7 @@ func TestRun(t *testing.T) { "test": {}, }, } - if err := Run(ctx, conf, nil, nil, nil, false, filter.ModeAdded, false); err == nil { + if err := Run(ctx, conf, nil, nil, nil, false, filter.ModeAdded, reviewdog.FailLevelNone); err == nil { t.Error("want error, got nil") } else { t.Log(err) @@ -72,7 +72,7 @@ func TestRun(t *testing.T) { }, }, } - if err := Run(ctx, conf, nil, nil, ds, false, filter.ModeAdded, false); err == nil { + if err := Run(ctx, conf, nil, nil, ds, false, filter.ModeAdded, reviewdog.FailLevelNone); err == nil { t.Error("want error, got nil") } else { t.Log(err) @@ -100,7 +100,7 @@ func TestRun(t *testing.T) { }, }, } - if err := Run(ctx, conf, nil, cs, ds, false, filter.ModeAdded, false); err != nil { + if err := Run(ctx, conf, nil, cs, ds, false, filter.ModeAdded, reviewdog.FailLevelNone); err != nil { t.Error(err) } want := "" @@ -130,7 +130,7 @@ func TestRun(t *testing.T) { }, }, } - if err := Run(ctx, conf, nil, cs, ds, false, filter.ModeAdded, false); err == nil { + if err := Run(ctx, conf, nil, cs, ds, false, filter.ModeAdded, reviewdog.FailLevelNone); err == nil { t.Error("want error, got nil") } else { t.Log(err) @@ -158,7 +158,7 @@ func TestRun(t *testing.T) { }, }, } - if err := Run(ctx, conf, nil, cs, ds, true, filter.ModeAdded, false); err == nil { + if err := Run(ctx, conf, nil, cs, ds, true, filter.ModeAdded, reviewdog.FailLevelNone); err == nil { t.Error("want error, got nil") } else { t.Log(err) @@ -193,7 +193,7 @@ func TestRun(t *testing.T) { }, }, } - if err := Run(ctx, conf, nil, cs, ds, false, filter.ModeAdded, false); err != nil { + if err := Run(ctx, conf, nil, cs, ds, false, filter.ModeAdded, reviewdog.FailLevelNone); err != nil { t.Error(err) } }) @@ -219,7 +219,7 @@ func TestRun(t *testing.T) { }, }, } - if err := Run(ctx, conf, nil, cs, ds, true, filter.ModeAdded, false); err != nil { + if err := Run(ctx, conf, nil, cs, ds, true, filter.ModeAdded, reviewdog.FailLevelNone); err != nil { t.Error(err) } want := "hi\n" @@ -255,7 +255,7 @@ func TestRun(t *testing.T) { }, }, } - if err := Run(ctx, conf, map[string]bool{"test2": true}, cs, ds, false, filter.ModeAdded, false); err != nil { + if err := Run(ctx, conf, map[string]bool{"test2": true}, cs, ds, false, filter.ModeAdded, reviewdog.FailLevelNone); err != nil { t.Error(err) } if called != 1 { @@ -288,7 +288,7 @@ func TestRun(t *testing.T) { }, }, } - if err := Run(ctx, conf, map[string]bool{"hoge": true}, cs, ds, false, filter.ModeAdded, false); err == nil { + if err := Run(ctx, conf, map[string]bool{"hoge": true}, cs, ds, false, filter.ModeAdded, reviewdog.FailLevelNone); err == nil { t.Error("got no error but want runner not found error") } }) diff --git a/reviewdog.go b/reviewdog.go index ed324275..5d3ee0c4 100644 --- a/reviewdog.go +++ b/reviewdog.go @@ -17,23 +17,23 @@ import ( // or linter, get diff and filter the results by diff, and report filtered // results. type Reviewdog struct { - toolname string - p parser.Parser - c CommentService - d DiffService - filterMode filter.Mode - failOnError bool + toolname string + p parser.Parser + c CommentService + d DiffService + filterMode filter.Mode + failLevel FailLevel } // NewReviewdog returns a new Reviewdog. -func NewReviewdog(toolname string, p parser.Parser, c CommentService, d DiffService, filterMode filter.Mode, failOnError bool) *Reviewdog { - return &Reviewdog{p: p, c: c, d: d, toolname: toolname, filterMode: filterMode, failOnError: failOnError} +func NewReviewdog(toolname string, p parser.Parser, c CommentService, d DiffService, filterMode filter.Mode, failLevel FailLevel) *Reviewdog { + return &Reviewdog{p: p, c: c, d: d, toolname: toolname, filterMode: filterMode, failLevel: failLevel} } // RunFromResult creates a new Reviewdog and runs it with check results. func RunFromResult(ctx context.Context, c CommentService, results []*rdf.Diagnostic, - filediffs []*diff.FileDiff, strip int, toolname string, filterMode filter.Mode, failOnError bool) error { - return (&Reviewdog{c: c, toolname: toolname, filterMode: filterMode, failOnError: failOnError}).runFromResult(ctx, results, filediffs, strip, failOnError) + filediffs []*diff.FileDiff, strip int, toolname string, filterMode filter.Mode, failLevel FailLevel) error { + return (&Reviewdog{c: c, toolname: toolname, filterMode: filterMode, failLevel: failLevel}).runFromResult(ctx, results, filediffs, strip, failLevel) } // Comment represents a reported result as a comment. @@ -67,14 +67,14 @@ type DiffService interface { } func (w *Reviewdog) runFromResult(ctx context.Context, results []*rdf.Diagnostic, - filediffs []*diff.FileDiff, strip int, failOnError bool) error { + filediffs []*diff.FileDiff, strip int, failLevel FailLevel) error { wd, err := os.Getwd() if err != nil { return err } checks := filter.FilterCheck(results, filediffs, strip, wd, w.filterMode) - hasViolations := false + shouldFail := false for _, check := range checks { comment := &Comment{ @@ -93,7 +93,7 @@ func (w *Reviewdog) runFromResult(ctx context.Context, results []*rdf.Diagnostic if err := w.c.Post(ctx, comment); err != nil { return err } - hasViolations = true + shouldFail = shouldFail || failLevel.ShouldFail(check.Diagnostic.GetSeverity()) } } @@ -103,8 +103,8 @@ func (w *Reviewdog) runFromResult(ctx context.Context, results []*rdf.Diagnostic } } - if failOnError && hasViolations { - return fmt.Errorf("input data has violations") + if shouldFail { + return fmt.Errorf("found at least one issue with severity greater than or equal to the given level: %s", failLevel.String()) } return nil @@ -127,5 +127,5 @@ func (w *Reviewdog) Run(ctx context.Context, r io.Reader) error { return fmt.Errorf("fail to parse diff: %w", err) } - return w.runFromResult(ctx, results, filediffs, w.d.Strip(), w.failOnError) + return w.runFromResult(ctx, results, filediffs, w.d.Strip(), w.failLevel) } diff --git a/reviewdog_test.go b/reviewdog_test.go index 5067e85a..4b612a19 100644 --- a/reviewdog_test.go +++ b/reviewdog_test.go @@ -50,7 +50,7 @@ golint.new.go:11:1: comment on exported function F2 should be of the form "F2 .. p := parser.NewErrorformatParser(efm) c := NewRawCommentWriter(os.Stdout) d := NewDiffString(difftext, 1) - app := NewReviewdog("tool name", p, c, d, filter.ModeAdded, false) + app := NewReviewdog("tool name", p, c, d, filter.ModeAdded, FailLevelDefault) app.Run(context.Background(), strings.NewReader(lintresult)) // Unordered output: // golint.new.go:5:5: exported var NewError1 should have comment or be unexported @@ -96,7 +96,7 @@ index 34cacb9..a727dd3 100644 efm, _ := errorformat.NewErrorformat([]string{`%f:%l:%c: %m`}) p := parser.NewErrorformatParser(efm) d := NewDiffString(difftext, 1) - app := NewReviewdog("tool name", p, c, d, filter.ModeAdded, false) + app := NewReviewdog("tool name", p, c, d, filter.ModeAdded, FailLevelDefault) app.Run(context.Background(), strings.NewReader(lintresult)) } @@ -129,7 +129,7 @@ golint.new.go:11:1: comment on exported function F2 should be of the form "F2 .. efm, _ := errorformat.NewErrorformat([]string{`%f:%l:%c: %m`}) p := parser.NewErrorformatParser(efm) d := NewDiffString(difftext, 1) - app := NewReviewdog("tool name", p, c, d, filter.ModeAdded, false) + app := NewReviewdog("tool name", p, c, d, filter.ModeAdded, FailLevelDefault) err := app.Run(context.Background(), strings.NewReader(lintresult)) if err != nil { @@ -165,7 +165,7 @@ golint.new.go:11:1: comment on exported function F2 should be of the form "F2 .. efm, _ := errorformat.NewErrorformat([]string{`%f:%l:%c: %m`}) p := parser.NewErrorformatParser(efm) d := NewDiffString(difftext, 1) - app := NewReviewdog("tool name", p, c, d, filter.ModeAdded, true) + app := NewReviewdog("tool name", p, c, d, filter.ModeAdded, FailLevelAny) err := app.Run(context.Background(), strings.NewReader(lintresult)) if err != nil && err.Error() != "input data has violations" { From 8052441205dc51660a03e1357f6317c9b71341e3 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Fri, 26 Jul 2024 21:42:08 +0900 Subject: [PATCH 2/8] Update CHANGELOG --- CHANGELOG.md | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c29152e2..b4564295 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,8 +14,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### :bug: Fixes - ... -### :rotating_light: Breaking changes -- ... +### :rotating_light: (Actually No) Breaking changes +- [#1854](https://github.com/reviewdog/reviewdog/pull/1854) `-fail-on-error` + flag is deprecated. Use `-fail-level=[none,any,error,warning,info]` flag + instead. You can reproduce the same behavior with `-fail-level=any` for most + reporters (reviewdog will exit with 1 if it find any severity level). + As for `github-[pr-]check` reporter you can reproduce the same behavior with + `-fail-level=error` ([#1846](https://github.com/reviewdog/reviewdog/issues/1846)). + + `-fail-on-error` is just deprecated and it's not removed yet. Please update + the flag while it's working. + + ## [v0.20.1] - 2024-07-10 @@ -47,6 +57,15 @@ We left both versions as-is so that it won't break existing clients who already - [#1809](https://github.com/reviewdog/reviewdog/pull/1809) Support posting comments outside diff context as a file comment in github-pr-review reporter +### :rotating_light: Breaking changes +- [#1846](https://github.com/reviewdog/reviewdog/issues/1846) the behavior of + `-fail-on-error` changed with `-github-[pr-]check`. Previously, reviewdog + didn't exit with 1 if it find issues with warning and info level. Now + reviewdog will exit with 1 with any level. + + The previous behavior can be restored with `-fail-level=error` flag with the + upcoming release. + ## [v0.18.1] - 2024-06-22 ### :bug: Fixes From 9bc9e31fabc24a3ea772d966fe3fae656570e069 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Fri, 26 Jul 2024 21:43:59 +0900 Subject: [PATCH 3/8] Use FailLevelError for -github-[pr-]check reporter with deprecated -fail-on-error --- cmd/reviewdog/main.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/reviewdog/main.go b/cmd/reviewdog/main.go index 5a466c2e..2a1d75ab 100644 --- a/cmd/reviewdog/main.go +++ b/cmd/reviewdog/main.go @@ -1037,7 +1037,12 @@ func failLevel(opt *option) reviewdog.FailLevel { if opt.failOnError { slog.Warn("reviewdog: -fail-on-error is deprecated. Use -fail-level=any, or -fail-level=error for github-[pr-]check reporter instead.") if opt.failLevel == reviewdog.FailLevelDefault { - return reviewdog.FailLevelAny + switch opt.reporter { + default: + return reviewdog.FailLevelAny + case "github-check", "github-pr-check": + return reviewdog.FailLevelError + } } } return opt.failLevel From c252e3ce3ecf9e3a44fbe0033d09b3b4676f2e00 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Fri, 26 Jul 2024 21:45:37 +0900 Subject: [PATCH 4/8] Update Fixes section in the change log --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4564295..743f143b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - ... ### :bug: Fixes -- ... +- [#1846](https://github.com/reviewdog/reviewdog/issues/1846) Unexpected error + exit code when using `-reporter=github-[pr-]check` with + `-fail-on-error=true`. See the below breaking changes section for more details. ### :rotating_light: (Actually No) Breaking changes - [#1854](https://github.com/reviewdog/reviewdog/pull/1854) `-fail-on-error` From 2c0dc58b38e602b7682bd9e887ba7b0f1d8a4b33 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Fri, 26 Jul 2024 21:49:14 +0900 Subject: [PATCH 5/8] Fix comments --- fail_level.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fail_level.go b/fail_level.go index 0a1b63db..eb737f7e 100644 --- a/fail_level.go +++ b/fail_level.go @@ -13,10 +13,15 @@ const ( // FailLevelDefault represents default mode, which means users doesn't specify // fail-level. Basically, it's same as FailLevelNone. FailLevelDefault FailLevel = iota + // FailLevelNone - Do not fail. FailLevelNone + // FailLevelAny - Fail with any severity. FailLevelAny + // FailLevelInfo - Fail with info or above severity. FailLevelInfo + // FailLevelWarning - Fail with warning or above severity. FailLevelWarning + // FailLevelError - Fail with error or above severity. FailLevelError ) From 1762e0a732afeeac29b4798534d1603196fe5649 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Fri, 26 Jul 2024 21:51:33 +0900 Subject: [PATCH 6/8] Add a link to CHANGELOG for the -fail-on-error deprecation log --- cmd/reviewdog/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/reviewdog/main.go b/cmd/reviewdog/main.go index 2a1d75ab..f905c3bc 100644 --- a/cmd/reviewdog/main.go +++ b/cmd/reviewdog/main.go @@ -1035,7 +1035,7 @@ func localDiffService(opt *option) (reviewdog.DiffService, error) { func failLevel(opt *option) reviewdog.FailLevel { if opt.failOnError { - slog.Warn("reviewdog: -fail-on-error is deprecated. Use -fail-level=any, or -fail-level=error for github-[pr-]check reporter instead.") + slog.Warn("reviewdog: -fail-on-error is deprecated. Use -fail-level=any, or -fail-level=error for github-[pr-]check reporter instead. See also https://github.com/reviewdog/reviewdog/blob/master/CHANGELOG.md") if opt.failLevel == reviewdog.FailLevelDefault { switch opt.reporter { default: From 160bcf7aef3defce06778431889c91ff84642106 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Fri, 26 Jul 2024 22:01:55 +0900 Subject: [PATCH 7/8] Update README --- README.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 1ded0e87..c246035c 100644 --- a/README.md +++ b/README.md @@ -904,12 +904,11 @@ $ REVIEWDOG_SKIP_DOGHOUSE=true REVIEWDOG_GITHUB_API_TOKEN="" reviewdog -r ``` ## Exit codes -By default reviewdog will return `0` as exit code even if it finds errors. -If `-fail-on-error` flag is passed, reviewdog exits with `1` when at least one error was found/reported. +By default (`-fail-level=none`) reviewdog will return `0` as exit code even if it finds errors. +reviewdog will exit with code 1 with `-fail-level=[any,info,warning,error]` flag if it finds at least 1 issue with severity greater than or equal to the given level. This can be helpful when you are using it as a step in your CI pipeline and want to mark the step failed if any error found by linter. -See also `-level` flag for [github-pr-check/github-check](#reporter-github-checks--reportergithub-pr-check) reporters. -reviewdog will exit with `1` if reported check status is `failure` as well if `-fail-on-error=true`. +You can also use `-level` flag to configure default report revel. ## Filter mode reviewdog filter results by diff and you can control how reviewdog filter results by `-filter-mode` flag. From aaf825220e9f0bf83387dad12ada3ff52fe3f102 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Fri, 26 Jul 2024 22:11:46 +0900 Subject: [PATCH 8/8] remove failLevel param from Reviewdog.runFromResult --- reviewdog.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/reviewdog.go b/reviewdog.go index 5d3ee0c4..9bc0b78f 100644 --- a/reviewdog.go +++ b/reviewdog.go @@ -33,7 +33,7 @@ func NewReviewdog(toolname string, p parser.Parser, c CommentService, d DiffServ // RunFromResult creates a new Reviewdog and runs it with check results. func RunFromResult(ctx context.Context, c CommentService, results []*rdf.Diagnostic, filediffs []*diff.FileDiff, strip int, toolname string, filterMode filter.Mode, failLevel FailLevel) error { - return (&Reviewdog{c: c, toolname: toolname, filterMode: filterMode, failLevel: failLevel}).runFromResult(ctx, results, filediffs, strip, failLevel) + return (&Reviewdog{c: c, toolname: toolname, filterMode: filterMode, failLevel: failLevel}).runFromResult(ctx, results, filediffs, strip) } // Comment represents a reported result as a comment. @@ -67,7 +67,7 @@ type DiffService interface { } func (w *Reviewdog) runFromResult(ctx context.Context, results []*rdf.Diagnostic, - filediffs []*diff.FileDiff, strip int, failLevel FailLevel) error { + filediffs []*diff.FileDiff, strip int) error { wd, err := os.Getwd() if err != nil { return err @@ -93,7 +93,7 @@ func (w *Reviewdog) runFromResult(ctx context.Context, results []*rdf.Diagnostic if err := w.c.Post(ctx, comment); err != nil { return err } - shouldFail = shouldFail || failLevel.ShouldFail(check.Diagnostic.GetSeverity()) + shouldFail = shouldFail || w.failLevel.ShouldFail(check.Diagnostic.GetSeverity()) } } @@ -104,7 +104,7 @@ func (w *Reviewdog) runFromResult(ctx context.Context, results []*rdf.Diagnostic } if shouldFail { - return fmt.Errorf("found at least one issue with severity greater than or equal to the given level: %s", failLevel.String()) + return fmt.Errorf("found at least one issue with severity greater than or equal to the given level: %s", w.failLevel.String()) } return nil @@ -127,5 +127,5 @@ func (w *Reviewdog) Run(ctx context.Context, r io.Reader) error { return fmt.Errorf("fail to parse diff: %w", err) } - return w.runFromResult(ctx, results, filediffs, w.d.Strip(), w.failLevel) + return w.runFromResult(ctx, results, filediffs, w.d.Strip()) }