Skip to content

Commit

Permalink
Some fixes.
Browse files Browse the repository at this point in the history
  • Loading branch information
i-sevostyanov committed Oct 18, 2020
1 parent 700f376 commit eca9728
Show file tree
Hide file tree
Showing 23 changed files with 93 additions and 83 deletions.
21 changes: 10 additions & 11 deletions cmd/reviewdog/main.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"context"
"crypto/tls"
"errors"
"flag"
Expand All @@ -16,7 +17,6 @@ import (
"text/tabwriter"

"golang.org/x/build/gerrit"
"golang.org/x/net/context" // "context"
"golang.org/x/oauth2"

"github.com/google/go-github/v32/github"
Expand Down Expand Up @@ -61,16 +61,15 @@ type option struct {
failOnError bool
}

// flags doc
const (
diffCmdDoc = `diff command (e.g. "git diff") for local reporter. Do not use --relative flag for git command.`
diffStripDoc = "strip NUM leading components from diff file names (equivalent to 'patch -p') (default is 1 for git diff)"
efmsDoc = `list of errorformat (https://github.com/reviewdog/errorformat)`
fDoc = `format name (run -list to see supported format name) for input. It's also used as tool name in review comment if -name is empty`
fDiffStripDoc = `option for -f=diff: strip NUM leading components from diff file names (equivalent to 'patch -p') (default is 1 for git diff)`
listDoc = `list supported pre-defined format names which can be used as -f arg`
nameDoc = `tool name in review comment. -f is used as tool name if -name is empty`
ciDoc = `[deprecated] reviewdog automatically get necessary data. See also -reporter for migration`
diffCmdDoc = `diff command (e.g. "git diff") for local reporter. Do not use --relative flag for git command.`
diffStripDoc = "strip NUM leading components from diff file names (equivalent to 'patch -p') (default is 1 for git diff)"
efmsDoc = `list of errorformat (https://github.com/reviewdog/errorformat)`
fDoc = `format name (run -list to see supported format name) for input. It's also used as tool name in review comment if -name is empty`
fDiffStripDoc = `option for -f=diff: strip NUM leading components from diff file names (equivalent to 'patch -p') (default is 1 for git diff)`
listDoc = `list supported pre-defined format names which can be used as -f arg`
nameDoc = `tool name in review comment. -f is used as tool name if -name is empty`

confDoc = `config file path`
runnersDoc = `comma separated runners name to run in config file. default: run all runners`
levelDoc = `report level currently used for github-pr-check reporter ("info","warning","error").`
Expand Down Expand Up @@ -405,7 +404,7 @@ func insecureSkipVerify() bool {
return os.Getenv("REVIEWDOG_INSECURE_SKIP_VERIFY") == "true"
}

func githubService(ctx context.Context, opt *option) (gs *githubservice.GitHubPullRequest, isPR bool, err error) {
func githubService(ctx context.Context, opt *option) (gs *githubservice.PullRequest, isPR bool, err error) {
token, err := nonEmptyEnv("REVIEWDOG_GITHUB_API_TOKEN")
if err != nil {
return nil, isPR, err
Expand Down
3 changes: 2 additions & 1 deletion doghouse/appengine/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import (
"net/url"
"strings"

"github.com/vvakame/sdlog/aelog"

"github.com/reviewdog/reviewdog/doghouse"
"github.com/reviewdog/reviewdog/doghouse/server"
"github.com/reviewdog/reviewdog/doghouse/server/ciutil"
"github.com/reviewdog/reviewdog/doghouse/server/storage"
"github.com/vvakame/sdlog/aelog"
)

type githubChecker struct {
Expand Down
2 changes: 1 addition & 1 deletion doghouse/appengine/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (g *GitHubHandler) HandleAuthCallback(w http.ResponseWriter, r *http.Reques

// Redirect.
redirURL := "/gh/"
if r, _ := g.redirURLStore.Get(r); err == nil {
if r, err := g.redirURLStore.Get(r); err == nil {
redirURL = string(r)
g.redirURLStore.Clear(w)
}
Expand Down
5 changes: 3 additions & 2 deletions doghouse/appengine/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ import (
"contrib.go.opencensus.io/exporter/stackdriver/propagation"
"github.com/haya14busa/secretbox"
"github.com/justinas/nosurf"
"github.com/reviewdog/reviewdog/doghouse/server/cookieman"
"github.com/reviewdog/reviewdog/doghouse/server/storage"
"go.opencensus.io/plugin/ochttp"
"go.opencensus.io/trace"

"github.com/reviewdog/reviewdog/doghouse/server/cookieman"
"github.com/reviewdog/reviewdog/doghouse/server/storage"
)

func mustCookieMan() *cookieman.CookieMan {
Expand Down
3 changes: 2 additions & 1 deletion doghouse/appengine/warmup.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ package main
import (
"net/http"

"github.com/reviewdog/reviewdog/doghouse/server/ciutil"
"github.com/vvakame/sdlog/aelog"

"github.com/reviewdog/reviewdog/doghouse/server/ciutil"
)

func warmupHandler(_ http.ResponseWriter, r *http.Request) {
Expand Down
4 changes: 2 additions & 2 deletions doghouse/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ func New(client *http.Client) *DogHouseClient {

// Check send check requests to doghouse.
func (c *DogHouseClient) Check(ctx context.Context, req *doghouse.CheckRequest) (*doghouse.CheckResponse, error) {
url := c.BaseURL.String() + "/check"
checkURL := c.BaseURL.String() + "/check"
b, err := json.Marshal(req)
if err != nil {
return nil, err
}
httpReq, err := http.NewRequest(http.MethodPost, url, bytes.NewReader(b))
httpReq, err := http.NewRequest(http.MethodPost, checkURL, bytes.NewReader(b))
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions filter/diff_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,11 @@ func (df *DiffFilter) ShouldReport(path string, lnum int) (bool, *diff.FileDiff,
file := df.difffiles[npath]
lines, ok := df.difflines[npath]
if !ok {
return (df.mode == ModeNoFilter), file, nil
return df.mode == ModeNoFilter, file, nil
}
line, ok := lines[lnum]
if !ok {
return (df.mode == ModeNoFilter || df.mode == ModeFile), file, nil
return df.mode == ModeNoFilter || df.mode == ModeFile, file, nil
}
return df.isSignificantLine(line), file, line
}
Expand Down
2 changes: 1 addition & 1 deletion filter/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func FilterCheck(results []*rdf.Diagnostic, diff []*diff.FileDiff, strip int,
endLine = startLine
}
check.InDiffContext = true
sourceLines := []string{}
var sourceLines []string
for l := startLine; l <= endLine; l++ {
shouldReport, difffile, diffline := df.ShouldReport(loc.GetPath(), l)
check.ShouldReport = check.ShouldReport || shouldReport
Expand Down
8 changes: 4 additions & 4 deletions filter/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ func TestFilterCheckByAddedLines(t *testing.T) {
}
filediffs, _ := diff.ParseMultiFile(strings.NewReader(diffContent))
got := FilterCheck(results, filediffs, 0, "", ModeAdded)
if diff := cmp.Diff(got, want, protocmp.Transform()); diff != "" {
t.Error(diff)
if value := cmp.Diff(got, want, protocmp.Transform()); value != "" {
t.Error(value)
}
}

Expand Down Expand Up @@ -231,8 +231,8 @@ func TestFilterCheckByDiffContext(t *testing.T) {
}
filediffs, _ := diff.ParseMultiFile(strings.NewReader(diffContent))
got := FilterCheck(results, filediffs, 0, "", ModeDiffContext)
if diff := cmp.Diff(got, want, protocmp.Transform()); diff != "" {
t.Error(diff)
if value := cmp.Diff(got, want, protocmp.Transform()); value != "" {
t.Error(value)
}
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ require (
github.com/xanzy/go-gitlab v0.38.1
go.opencensus.io v0.22.5
golang.org/x/build v0.0.0-20200616162219-07bebbe343e9
golang.org/x/net v0.0.0-20201016165138-7b1cca2348c0
golang.org/x/net v0.0.0-20201016165138-7b1cca2348c0 // indirect
golang.org/x/oauth2 v0.0.0-20200902213428-5d25da1a8d43
golang.org/x/sync v0.0.0-20201008141435-b3e1573b7520
google.golang.org/protobuf v1.25.0
Expand Down
2 changes: 1 addition & 1 deletion parser/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (p *DiffParser) Parse(r io.Reader) ([]*rdf.Diagnostic, error) {
if err != nil {
return nil, fmt.Errorf("fail to parse diff: %w", err)
}
diagnostics := []*rdf.Diagnostic{}
var diagnostics []*rdf.Diagnostic
for _, fdiff := range filediffs {
path := filter.NormalizeDiffPath(fdiff.PathNew, p.strip)
for _, hunk := range fdiff.Hunks {
Expand Down
1 change: 1 addition & 0 deletions parser/errorformat.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strings"

"github.com/reviewdog/errorformat"

"github.com/reviewdog/reviewdog/proto/rdf"
)

Expand Down
1 change: 1 addition & 0 deletions parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io"

"github.com/reviewdog/errorformat/fmts"

"github.com/reviewdog/reviewdog/proto/rdf"
)

Expand Down
3 changes: 2 additions & 1 deletion parser/rdjson.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import (
"io"
"io/ioutil"

"github.com/reviewdog/reviewdog/proto/rdf"
"google.golang.org/protobuf/encoding/protojson"

"github.com/reviewdog/reviewdog/proto/rdf"
)

var _ Parser = &RDJSONParser{}
Expand Down
3 changes: 2 additions & 1 deletion parser/rdjsonl.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import (
"fmt"
"io"

"github.com/reviewdog/reviewdog/proto/rdf"
"google.golang.org/protobuf/encoding/protojson"

"github.com/reviewdog/reviewdog/proto/rdf"
)

var _ Parser = &RDJSONLParser{}
Expand Down
1 change: 1 addition & 0 deletions reviewdog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"testing"

"github.com/reviewdog/errorformat"

"github.com/reviewdog/reviewdog/filter"
"github.com/reviewdog/reviewdog/parser"
)
Expand Down
3 changes: 2 additions & 1 deletion service/gerrit/change_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import (
"path/filepath"
"sync"

"golang.org/x/build/gerrit"

"github.com/reviewdog/reviewdog"
"github.com/reviewdog/reviewdog/service/serviceutil"
"golang.org/x/build/gerrit"
)

var _ reviewdog.CommentService = &ChangeReviewCommenter{}
Expand Down
3 changes: 2 additions & 1 deletion service/gerrit/change_review_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"golang.org/x/build/gerrit"

"github.com/reviewdog/reviewdog"
"github.com/reviewdog/reviewdog/filter"
"github.com/reviewdog/reviewdog/proto/rdf"
"golang.org/x/build/gerrit"
)

func TestChangeReviewCommenter_Post_Flush(t *testing.T) {
Expand Down
34 changes: 17 additions & 17 deletions service/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import (
"github.com/reviewdog/reviewdog/service/serviceutil"
)

var _ reviewdog.CommentService = &GitHubPullRequest{}
var _ reviewdog.DiffService = &GitHubPullRequest{}
var _ reviewdog.CommentService = &PullRequest{}
var _ reviewdog.DiffService = &PullRequest{}

const maxCommentsPerRequest = 30

Expand All @@ -28,12 +28,12 @@ const (
invalidSuggestionPost = "</details>"
)

// GitHubPullRequest is a comment and diff service for GitHub PullRequest.
// PullRequest is a comment and diff service for GitHub PullRequest.
//
// API:
// https://developer.github.com/v3/pulls/comments/#create-a-comment
// POST /repos/:owner/:repo/pulls/:number/comments
type GitHubPullRequest struct {
type PullRequest struct {
cli *github.Client
owner string
repo string
Expand All @@ -49,14 +49,14 @@ type GitHubPullRequest struct {
wd string
}

// NewGitHubPullRequest returns a new GitHubPullRequest service.
// GitHubPullRequest service needs git command in $PATH.
func NewGitHubPullRequest(cli *github.Client, owner, repo string, pr int, sha string) (*GitHubPullRequest, error) {
// NewGitHubPullRequest returns a new PullRequest service.
// PullRequest service needs git command in $PATH.
func NewGitHubPullRequest(cli *github.Client, owner, repo string, pr int, sha string) (*PullRequest, error) {
workDir, err := serviceutil.GitRelWorkdir()
if err != nil {
return nil, fmt.Errorf("GitHubPullRequest needs 'git' command: %w", err)
return nil, fmt.Errorf("PullRequest needs 'git' command: %w", err)
}
return &GitHubPullRequest{
return &PullRequest{
cli: cli,
owner: owner,
repo: repo,
Expand All @@ -68,7 +68,7 @@ func NewGitHubPullRequest(cli *github.Client, owner, repo string, pr int, sha st

// Post accepts a comment and holds it. Flush method actually posts comments to
// GitHub in parallel.
func (g *GitHubPullRequest) Post(_ context.Context, c *reviewdog.Comment) error {
func (g *PullRequest) Post(_ context.Context, c *reviewdog.Comment) error {
c.Result.Diagnostic.GetLocation().Path = filepath.ToSlash(filepath.Join(g.wd,
c.Result.Diagnostic.GetLocation().GetPath()))
g.muComments.Lock()
Expand All @@ -78,7 +78,7 @@ func (g *GitHubPullRequest) Post(_ context.Context, c *reviewdog.Comment) error
}

// Flush posts comments which has not been posted yet.
func (g *GitHubPullRequest) Flush(ctx context.Context) error {
func (g *PullRequest) Flush(ctx context.Context) error {
g.muComments.Lock()
defer g.muComments.Unlock()

Expand All @@ -88,7 +88,7 @@ func (g *GitHubPullRequest) Flush(ctx context.Context) error {
return g.postAsReviewComment(ctx)
}

func (g *GitHubPullRequest) postAsReviewComment(ctx context.Context) error {
func (g *PullRequest) postAsReviewComment(ctx context.Context) error {
comments := make([]*github.DraftReviewComment, 0, len(g.postComments))
remaining := make([]*reviewdog.Comment, 0)
for _, c := range g.postComments {
Expand Down Expand Up @@ -163,7 +163,7 @@ func githubCommentLine(c *reviewdog.Comment) int {
return int(line)
}

func (g *GitHubPullRequest) remainingCommentsSummary(remaining []*reviewdog.Comment) string {
func (g *PullRequest) remainingCommentsSummary(remaining []*reviewdog.Comment) string {
if len(remaining) == 0 {
return ""
}
Expand All @@ -187,7 +187,7 @@ func (g *GitHubPullRequest) remainingCommentsSummary(remaining []*reviewdog.Comm
return sb.String()
}

func (g *GitHubPullRequest) setPostedComment(ctx context.Context) error {
func (g *PullRequest) setPostedComment(ctx context.Context) error {
g.postedcs = make(commentutil.PostedComments)
cs, err := g.comment(ctx)
if err != nil {
Expand All @@ -203,7 +203,7 @@ func (g *GitHubPullRequest) setPostedComment(ctx context.Context) error {
}

// Diff returns a diff of PullRequest.
func (g *GitHubPullRequest) Diff(ctx context.Context) ([]byte, error) {
func (g *PullRequest) Diff(ctx context.Context) ([]byte, error) {
opt := github.RawOptions{Type: github.Diff}
d, _, err := g.cli.PullRequests.GetRaw(ctx, g.owner, g.repo, g.pr, opt)
if err != nil {
Expand All @@ -213,11 +213,11 @@ func (g *GitHubPullRequest) Diff(ctx context.Context) ([]byte, error) {
}

// Strip returns 1 as a strip of git diff.
func (g *GitHubPullRequest) Strip() int {
func (g *PullRequest) Strip() int {
return 1
}

func (g *GitHubPullRequest) comment(ctx context.Context) ([]*github.PullRequestComment, error) {
func (g *PullRequest) comment(ctx context.Context) ([]*github.PullRequestComment, error) {
// https://developer.github.com/v3/guides/traversing-with-pagination/
opts := &github.PullRequestListCommentsOptions{
ListOptions: github.ListOptions{
Expand Down
1 change: 1 addition & 0 deletions service/github/githubutils/comment_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"sync"

"github.com/haya14busa/go-actions-toolkit/core"

"github.com/reviewdog/reviewdog"
"github.com/reviewdog/reviewdog/proto/rdf"
)
Expand Down
Loading

0 comments on commit eca9728

Please sign in to comment.