Skip to content

Commit

Permalink
fix report type for with correct platform
Browse files Browse the repository at this point in the history
  • Loading branch information
CarlJi committed Nov 27, 2024
1 parent 68b541c commit 568d241
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 47 deletions.
4 changes: 2 additions & 2 deletions clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,9 +361,9 @@ func (b *GitConfigBuilder) buildGitHubAuth() GitAuth {
}
}

if b.server.gitHubAccessToken != "" {
if b.server.gitHubPersonalAccessToken != "" {
return GitAuth{
GitHubAccessToken: b.server.gitHubAccessToken,
GitHubAccessToken: b.server.gitHubPersonalAccessToken,
}
}

Expand Down
35 changes: 22 additions & 13 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,10 @@ type Refs struct {
}

type GlobalConfig struct {
// GithubReportType/GitlabReportType is the format of the report, will be used if linterConfig.ReportFormat is empty.
// GitHubReportType/GitlabReportType is the format of the report, will be used if linterConfig.ReportFormat is empty.
// e.g. "github_checks", "github_pr_review"
GithubReportType ReportType `json:"githubReportType,omitempty"`
GitlabReportType ReportType `json:"gitlabReportType,omitempty"`
GitHubReportType ReportType `json:"githubReportType,omitempty"`
GitLabReportType ReportType `json:"gitlabReportType,omitempty"`

// GolangciLintConfig is the path of golangci-lint config file to run golangci-lint globally.
// if not empty, use the config to run golangci-lint.
Expand Down Expand Up @@ -236,8 +236,8 @@ func NewConfig(conf string) (Config, error) {
}

// set default value
if c.GlobalDefaultConfig.GithubReportType == "" {
c.GlobalDefaultConfig.GithubReportType = GithubPRReview
if c.GlobalDefaultConfig.GitHubReportType == "" {
c.GlobalDefaultConfig.GitHubReportType = GitHubMixType
}

// check golangci-lint config path
Expand Down Expand Up @@ -279,10 +279,10 @@ func (c Config) GetLinterConfig(org, repo, ln string, repoType Platform) Linter
Repo: repo,
}
if repoType == GitLab {
linter.ReportType = c.GlobalDefaultConfig.GithubReportType
linter.ReportType = c.GlobalDefaultConfig.GitLabReportType
}
if repoType == GitHub {
linter.ReportType = c.GlobalDefaultConfig.GithubReportType
linter.ReportType = c.GlobalDefaultConfig.GitHubReportType
}

// set golangci-lint config path if exists
Expand Down Expand Up @@ -421,14 +421,23 @@ func applyCustomLintersConfig(legacy Linter, custom CustomLinter) Linter {
type ReportType string

const (
GithubCheckRuns ReportType = "github_check_run"
GithubPRReview ReportType = "github_pr_review"
GitlabComment ReportType = "gitlab_mr_comment"
GitlabCommentAndDiscussion ReportType = "gitlab_mr_comment_discussion"
// GithubMixType is the type of the report that mix the github_check_run and github_pr_review.
// GitHubCheckRuns is the type of the report that use github check run to report the lint results.
// to use this report type, the auth must be github app since github_check_run is only supported on github app.
GitHubCheckRuns ReportType = "github_check_run"
// GitHubPRReview is the type of the report that use github pull request review to report the lint results.
GitHubPRReview ReportType = "github_pr_review"
// default report type for github
// GitHubMixType is the type of the report that mix the github_check_run and github_pr_review.
// which use the github_check_run to report all lint results as a check run summary,
// but use the github_pr_review to report top 10 lint results to pull request review comments at most.
GithubMixType ReportType = "github_mix"
// to use this report type, the auth must be github app since github_check_run is only supported on github app.
GitHubMixType ReportType = "github_mix"

// GitLabComment is the type of the report that use gitlab merge request comment to report the lint results.
GitLabComment ReportType = "gitlab_mr_comment"
// GitLabCommentAndDiscussion is the type of the report that use gitlab merge request comment and discussion to report the lint results.
GitLabCommentAndDiscussion ReportType = "gitlab_mr_comment_discussion"

// for debug and testing.
Quiet ReportType = "quiet"
)
Expand Down
10 changes: 5 additions & 5 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestConfig(t *testing.T) {
rawConfig: ``,
expected: Config{
GlobalDefaultConfig: GlobalConfig{
GithubReportType: GithubPRReview,
GitHubReportType: GitHubMixType,
},
},
},
Expand Down Expand Up @@ -90,7 +90,7 @@ issueReferences:
`,
expected: Config{
GlobalDefaultConfig: GlobalConfig{
GithubReportType: GithubCheckRuns,
GitHubReportType: GitHubCheckRuns,
},
CustomRepos: map[string]RepoConfig{
"qbox": {
Expand Down Expand Up @@ -198,7 +198,7 @@ customRepos: # custom config for specific orgs or repos
`,
expected: Config{
GlobalDefaultConfig: GlobalConfig{
GithubReportType: GithubCheckRuns,
GitHubReportType: GitHubCheckRuns,
GolangCiLintConfig: "linters-config/.golangci.yml",
},
CustomRepos: map[string]RepoConfig{
Expand Down Expand Up @@ -252,8 +252,8 @@ customRepos: # custom config for specific orgs or repos
t.Errorf("expected no error, got %v", err)
}

if c.GlobalDefaultConfig.GithubReportType != tc.expected.GlobalDefaultConfig.GithubReportType {
t.Errorf("expected %v, got %v", tc.expected.GlobalDefaultConfig.GithubReportType, c.GlobalDefaultConfig.GithubReportType)
if c.GlobalDefaultConfig.GitHubReportType != tc.expected.GlobalDefaultConfig.GitHubReportType {
t.Errorf("expected %v, got %v", tc.expected.GlobalDefaultConfig.GitHubReportType, c.GlobalDefaultConfig.GitHubReportType)
}

if !strings.HasSuffix(c.GlobalDefaultConfig.GolangCiLintConfig, tc.expected.GlobalDefaultConfig.GolangCiLintConfig) {
Expand Down
4 changes: 2 additions & 2 deletions internal/linters/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ type Agent struct {
// getMsgFormat returns the message format based on report type.
func getMsgFormat(format config.ReportType) string {
switch format {
case config.GithubPRReview, config.GithubMixType:
case config.GitHubPRReview, config.GitHubMixType:
return "[%s](%s)"
default:
return "%s\nmore info: %s"
Expand Down Expand Up @@ -111,7 +111,7 @@ func (a *Agent) processOutput(ctx context.Context, output LinterOutput, ref conf
newOutput.TypedMessage = fmt.Sprintf(msgFormat, output.Message, ref.URL)

// Add issue content for PR review formats
if a.LinterConfig.ReportType == config.GithubPRReview || a.LinterConfig.ReportType == config.GithubMixType {
if a.LinterConfig.ReportType == config.GitHubPRReview || a.LinterConfig.ReportType == config.GitHubMixType {
if content, err := a.getIssueContent(ctx, ref); err == nil {
newOutput.TypedMessage += fmt.Sprintf(ReferenceFooter, content)
}
Expand Down
10 changes: 5 additions & 5 deletions internal/linters/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestApplyTypedMessageByIssueReferences(t *testing.T) {
}{
{
name: "GithubCheckRuns format",
reportFormat: config.GithubCheckRuns,
reportFormat: config.GitHubCheckRuns,
lintResults: map[string][]LinterOutput{
"file1.go": {
{
Expand Down Expand Up @@ -51,7 +51,7 @@ func TestApplyTypedMessageByIssueReferences(t *testing.T) {
},
{
name: "GithubPRReview format",
reportFormat: config.GithubPRReview,
reportFormat: config.GitHubPRReview,
lintResults: map[string][]LinterOutput{
"file2.go": {
{
Expand Down Expand Up @@ -79,7 +79,7 @@ func TestApplyTypedMessageByIssueReferences(t *testing.T) {
},
{
name: "GithubMixType format",
reportFormat: config.GithubMixType,
reportFormat: config.GitHubMixType,
lintResults: map[string][]LinterOutput{
"file3.go": {
{
Expand Down Expand Up @@ -107,7 +107,7 @@ func TestApplyTypedMessageByIssueReferences(t *testing.T) {
},
{
name: "No matching issue reference",
reportFormat: config.GithubCheckRuns,
reportFormat: config.GitHubCheckRuns,
lintResults: map[string][]LinterOutput{
"file4.go": {
{
Expand All @@ -134,7 +134,7 @@ func TestApplyTypedMessageByIssueReferences(t *testing.T) {
},
{
name: "Multiple files and issues",
reportFormat: config.GithubMixType,
reportFormat: config.GitHubMixType,
lintResults: map[string][]LinterOutput{
"file5.go": {
{
Expand Down
2 changes: 1 addition & 1 deletion internal/linters/go/gofmt/gofmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func gofmtHandler(ctx context.Context, a linters.Agent) error {

// Since GitHub's check run feature does not have the suggestion functionality, GitHub PR review is fixed used to display gofmt reports.
// Details: https://github.com/qiniu/reviewbot/issues/166
a.LinterConfig.ReportType = config.GithubPRReview
a.LinterConfig.ReportType = config.GitHubPRReview

executor, err := NewgofmtExecutor(a.LinterConfig.WorkDir)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions internal/linters/providergithub.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ func (g *GithubProvider) Report(ctx context.Context, a Agent, lintResults map[st
orgRepo := fmt.Sprintf("%s/%s", org, repo)

switch a.LinterConfig.ReportType {
case config.GithubCheckRuns:
case config.GitHubCheckRuns:
check := newBaseCheckRun(a, lintResults)
ch, err := g.CreateCheckRun(ctx, org, repo, check)
if err != nil {
Expand All @@ -308,7 +308,7 @@ func (g *GithubProvider) Report(ctx context.Context, a Agent, lintResults map[st
log.Infof("[%s] create check run success, HTML_URL: %v", linterName, ch.GetHTMLURL())

metric.NotifyWebhookByText(ConstructGotchaMsg(linterName, a.Provider.GetCodeReviewInfo().URL, ch.GetHTMLURL(), lintResults))
case config.GithubPRReview:
case config.GitHubPRReview:
comments, err := g.ProcessComments(ctx, a, lintResults)
if err != nil {
log.Errorf("failed to process need to add comments: %v", err)
Expand All @@ -326,7 +326,7 @@ func (g *GithubProvider) Report(ctx context.Context, a Agent, lintResults map[st
}
log.Infof("[%s] add %d comments for this PR %d (%s) \n", linterName, len(addedCmts), num, orgRepo)
metric.NotifyWebhookByText(ConstructGotchaMsg(linterName, a.Provider.GetCodeReviewInfo().URL, addedCmts[0].GetHTMLURL(), lintResults))
case config.GithubMixType:
case config.GitHubMixType:
// report all lint results as a check run summary, but not annotations
check := newMixCheckRun(a, lintResults)
ch, err := g.CreateCheckRun(ctx, org, repo, check)
Expand Down
10 changes: 5 additions & 5 deletions internal/linters/providergitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,14 +242,14 @@ func reportFormatMatCheck(gc *gitlab.Client, reportFormat config.ReportType) (re
v2, _ := version.NewVersion("10.8")
if reportFormat != "" {
if v1.LessThan(v2) {
return config.GitlabComment
return config.GitLabComment
}
return reportFormat
}
if v1.LessThan(v2) {
return config.GitlabComment
return config.GitLabComment
}
return config.GitlabCommentAndDiscussion
return config.GitLabCommentAndDiscussion
}

func (g *GitlabProvider) HandleComments(ctx context.Context, outputs map[string][]LinterOutput) error {
Expand All @@ -268,7 +268,7 @@ func (g *GitlabProvider) Report(ctx context.Context, a Agent, lintResults map[st
orgRepo := fmt.Sprintf("%s/%s", org, repo)
reportFormat := reportFormatMatCheck(g.GitLabClient, a.LinterConfig.ReportType)
switch reportFormat {
case config.GitlabCommentAndDiscussion:
case config.GitLabCommentAndDiscussion:
// list MR comments
var pid = g.MergeRequestEvent.ObjectAttributes.TargetProjectID
existedComments, err := g.ListMergeRequestsComments(ctx, g.GitLabClient, org, repo, num, pid)
Expand Down Expand Up @@ -329,7 +329,7 @@ func (g *GitlabProvider) Report(ctx context.Context, a Agent, lintResults map[st
}
log.Infof("[%s] add %d comments for this PR %d (%s) \n", linterName, len(addedDis), num, orgRepo)
metric.NotifyWebhookByText(ConstructGotchaMsg(linterName, g.MergeRequestEvent.Project.WebURL, "", lintResults))
case config.GitlabComment:
case config.GitLabComment:
var pid = g.MergeRequestEvent.ObjectAttributes.TargetProjectID
existedComments, err := g.ListMergeRequestsComments(ctx, g.GitLabClient, org, repo, num, pid)
if err != nil {
Expand Down
12 changes: 6 additions & 6 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ type options struct {
gitLabHost string

// support github
gitHubAccessToken string
gitHubAppID int64
gitHubAppInstallationID int64
gitHubAppPrivateKey string
gitHubPersonalAccessToken string
gitHubAppID int64
gitHubAppInstallationID int64
gitHubAppPrivateKey string

// log storage dir for local storage
logDir string
Expand Down Expand Up @@ -112,7 +112,7 @@ func gatherOptions() options {
fs.StringVar(&o.kubeConfig, "kube-config", "", "kube config file")

// github related
fs.StringVar(&o.gitHubAccessToken, "github.access-token", "", "personal github access token")
fs.StringVar(&o.gitHubPersonalAccessToken, "github.personal-access-token", "", "personal github access token")
fs.Int64Var(&o.gitHubAppID, "github.app-id", 0, "github app id")
fs.Int64Var(&o.gitHubAppInstallationID, "github.app-installation-id", 0, "github app installation id")
fs.StringVar(&o.gitHubAppPrivateKey, "github.app-private-key", "", "github app private key")
Expand Down Expand Up @@ -242,7 +242,7 @@ func main() {
kubeConfig: o.kubeConfig,
gitLabHost: o.gitLabHost,
gitLabPersonalAccessToken: o.gitLabPersonalAccessToken,
gitHubAccessToken: o.gitHubAccessToken,
gitHubPersonalAccessToken: o.gitHubPersonalAccessToken,
}

// github app
Expand Down
10 changes: 5 additions & 5 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ type Server struct {
// support github app model
// gitHubAppID int64
// gitHubAppPrivateKey string
gitHubAppAuth *GitHubAppAuth
gitHubAccessToken string
gitHubAppAuth *GitHubAppAuth
gitHubPersonalAccessToken string

// token cache
githubAppTokenCache *githubAppTokenCache
Expand Down Expand Up @@ -253,7 +253,7 @@ func (s *Server) handleCodeRequestEvent(ctx context.Context, info *codeRequestIn
log := lintersutil.FromContext(ctx)

for name, fn := range linters.TotalPullRequestHandlers() {
linterConfig := s.config.GetLinterConfig(info.org, info.repo, name, config.GitHub)
linterConfig := s.config.GetLinterConfig(info.org, info.repo, name, info.platform)

// skip if linter is not enabled
if linterConfig.Enable != nil && !*linterConfig.Enable {
Expand Down Expand Up @@ -608,13 +608,13 @@ func (s *Server) githubAppClient(installationID int64) *github.Client {

func (s *Server) githubAccessTokenClient() *github.Client {
gc := github.NewClient(httpcache.NewMemoryCacheTransport().Client())
gc.WithAuthToken(s.gitHubAccessToken)
gc.WithAuthToken(s.gitHubPersonalAccessToken)
return gc
}

// GithubClient returns a github client.
func (s *Server) GithubClient(installationID int64) *github.Client {
if s.gitHubAccessToken != "" {
if s.gitHubPersonalAccessToken != "" {
return s.githubAccessTokenClient()
}
return s.githubAppClient(installationID)
Expand Down

0 comments on commit 568d241

Please sign in to comment.