Skip to content

Commit

Permalink
fix(checkpatch): Use checkpatch conf file if available
Browse files Browse the repository at this point in the history
Signed-off-by: Cezar Craciunoiu <[email protected]>
  • Loading branch information
craciunoiuc committed Apr 29, 2024
1 parent 1284113 commit c72f58c
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 24 deletions.
40 changes: 21 additions & 19 deletions cmd/governctl/pr/check/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ import (
)

type Patch struct {
CommitterEmail string `long:"committer-email" short:"e" env:"GOVERN_COMMITTER_EMAIL" usage:"Set the Git committer author's email"`
CommiterGlobal bool `long:"committer-global" env:"GOVERN_COMMITTER_GLOBAL" usage:"Set the Git committer author's email/name globally"`
CommitterName string `long:"committer-name" short:"n" env:"GOVERN_COMMITTER_NAME" usage:"Set the Git committer author's name"`
Output string `long:"output" short:"o" env:"GOVERN_OUTPUT" usage:"Set the output format of choice [table, html, json, yaml]" default:"table"`
CheckpatchScript string `long:"checkpatch-script" env:"GOVERN_CHECKPATCH_SCRIPT" usage:"Use an existing checkpatch.pl script"`
BaseBranch string `long:"base" env:"GOVERN_BASE_BRANCH" usage:"Set the base branch name that the PR will be rebased onto"`
Ignores []string `long:"ignore" env:"GOVERN_IGNORES" usage:"Ignore one or many checkpatch checks"`
CommitterEmail string `long:"committer-email" short:"e" env:"GOVERN_COMMITTER_EMAIL" usage:"Set the Git committer author's email"`
CommiterGlobal bool `long:"committer-global" env:"GOVERN_COMMITTER_GLOBAL" usage:"Set the Git committer author's email/name globally"`
CommitterName string `long:"committer-name" short:"n" env:"GOVERN_COMMITTER_NAME" usage:"Set the Git committer author's name"`
Output string `long:"output" short:"o" env:"GOVERN_OUTPUT" usage:"Set the output format of choice [table, html, json, yaml]" default:"table"`
CheckpatchScript string `long:"checkpatch-script" env:"GOVERN_CHECKPATCH_SCRIPT" usage:"Use an existing checkpatch.pl script"`
CheckpatchConf string `long:"checkpatch-conf" env:"GOVERN_CHECKPATCH_CONF" usage:"Use an existing checkpatch.conf file"`
BaseBranch string `long:"base" env:"GOVERN_BASE_BRANCH" usage:"Set the base branch name that the PR will be rebased onto"`
}

const (
Expand Down Expand Up @@ -65,16 +65,7 @@ func NewPatch() *cobra.Command {
}

func (opts *Patch) Run(ctx context.Context, args []string) error {
if len(opts.Ignores) == 0 {
opts.Ignores = []string{
"FILE_PATH_CHANGES",
"OBSOLETE",
"ASSIGN_IN_IF",
"NEW_TYPEDEFS",
"EMAIL_SUBJECT",
"AVOID_BUG",
}
}
var extraIgnores []string

ghOrg, ghRepo, ghPrId, err := cmdutils.ParseOrgRepoAndPullRequestArgs(args)
if err != nil {
Expand Down Expand Up @@ -114,7 +105,7 @@ func (opts *Patch) Run(ctx context.Context, args []string) error {

ignoreList := strings.SplitN(line, checkpatchIgnore, 2)[1]
for _, i := range strings.Split(ignoreList, ",") {
opts.Ignores = append(opts.Ignores,
extraIgnores = append(extraIgnores,
strings.ToUpper(strings.TrimSpace(i)),
)
}
Expand All @@ -133,6 +124,16 @@ func (opts *Patch) Run(ctx context.Context, args []string) error {
return fmt.Errorf("could not access checkpatch script at '%s': %w", opts.CheckpatchScript, err)
}

if opts.CheckpatchConf == "" {
opts.CheckpatchConf = filepath.Join(
pull.LocalRepo(),
".checkpatch.conf",
)
}
if _, err := os.Stat(opts.CheckpatchConf); err != nil {
return fmt.Errorf("could not access checkpatch configuration at '%s': %w", opts.CheckpatchConf, err)
}

cs := iostreams.G(ctx).ColorScheme()

topts := []tableprinter.TablePrinterOption{
Expand Down Expand Up @@ -174,8 +175,9 @@ func (opts *Patch) Run(ctx context.Context, args []string) error {

check, err := checkpatch.NewCheckpatch(ctx,
patch.Filename,
checkpatch.WithIgnore(opts.Ignores...),
checkpatch.WithIgnore(extraIgnores...),
checkpatch.WithCheckpatchScriptPath(opts.CheckpatchScript),
checkpatch.WithCheckpatchConfPath(opts.CheckpatchConf),
checkpatch.WithStderr(log.G(ctx).WriterLevel(logrus.TraceLevel)),
)
if err != nil {
Expand Down
21 changes: 16 additions & 5 deletions internal/checkpatch/checkpatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type Patch struct {
notes []*Note
stderr io.Writer
script string
conf string
}

type NoteLevel string
Expand Down Expand Up @@ -70,21 +71,31 @@ func NewCheckpatch(ctx context.Context, file string, opts ...PatchOption) (*Patc
patch.script = "checkpatch.pl"
}

if patch.conf == "" {
patch.conf = ".checkpatch.conf"
}

args := []string{
"--color=never",
"--show-types",
"--no-tree",
"--strict",
"--max-line-length=80",
file,
}

// Add options from the conf file in the PR
content, err := os.ReadFile(patch.conf)
if err != nil {
return nil, fmt.Errorf("could not read checkpatch configuration file: %w", err)
}
args = append(args, strings.Split(strings.TrimSpace(string(content)), "\n")...)

// Extra ignores from the commits.
if len(patch.ignores) > 0 {
args = append(args,
"--ignore",
strings.Join(patch.ignores, ","),
)
}

args = append(args, file)

c := exec.Command(patch.script, args...)
c.Env = os.Environ()

Expand Down
8 changes: 8 additions & 0 deletions internal/checkpatch/checkpatch_option.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,11 @@ func WithCheckpatchScriptPath(path string) PatchOption {
return nil
}
}

// WithCheckpatchConf sets the checkpatch configuration to use.
func WithCheckpatchConfPath(conf string) PatchOption {
return func(patch *Patch) error {
patch.conf = conf
return nil
}
}

0 comments on commit c72f58c

Please sign in to comment.