Skip to content

travis: fix DCO validation for merges #170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mrunalp merged 2 commits into from
Sep 10, 2015
Merged

travis: fix DCO validation for merges #170

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
041eb73
travis: fix DCO validation for merges
vbatts Sep 10, 2015
5a654b9
.tools: cleanup the commit entry
vbatts Sep 10, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 39 additions & 88 deletions .tools/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,14 @@ import (
var DefaultRules = []ValidateRule{
func(c CommitEntry) (vr ValidateResult) {
vr.CommitEntry = c
if len(strings.Split(c["parent"], " ")) > 1 {
vr.Pass = true
vr.Msg = "merge commits do not require DCO"
return vr
}

hasValid := false
for _, line := range strings.Split(c.Body, "\n") {
for _, line := range strings.Split(c["body"], "\n") {
if validDCO.MatchString(line) {
hasValid = true
}
Expand Down Expand Up @@ -63,14 +69,14 @@ func main() {

results := ValidateResults{}
for _, commit := range c {
fmt.Printf(" * %s %s ... ", commit.AbbreviatedCommit, commit.Subject)
fmt.Printf(" * %s %s ... ", commit["abbreviated_commit"], commit["subject"])
vr := ValidateCommit(commit, DefaultRules)
results = append(results, vr...)
if _, fail := vr.PassFail(); fail == 0 {
fmt.Println("PASS")
if *flVerbose {
for _, r := range vr {
if !r.Pass {
if r.Pass {
Copy link
Contributor

@wking wking Sep 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, sorry I missed this while reviewing #167. But we can probably drop this check entirely, since we only hit this code when fail == 0.

Copy link
Member Author

@vbatts vbatts Sep 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not true. this is so that you can see the messages for passed tests as well.

Copy link
Contributor

@wking wking Sep 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Thu, Sep 10, 2015 at 06:25:33AM -0700, Vincent Batts wrote:

  •               if !r.Pass {

  •               if r.Pass {

not true. this is so that you can see the messages for passed tests
as well.

You have (from #167):

if _, fail := vr.PassFail(); fail == 0 {
  fmt.Println("PASS")
  if *flVerbose {
    for _, r := range vr {
      if !r.Pass {
        fmt.Printf("  - %s\n", r.Msg)
      }
    }
  }
} else {
  fmt.Println("FAIL")
  // default, only print out failed validations
  for _, r := range vr {
    if !r.Pass {
      fmt.Printf("  - %s\n", r.Msg)
    }
  }
}

The fix you're proposing here would be the same as:

if _, fail := vr.PassFail(); fail == 0 {
  fmt.Println("PASS")
  if *flVerbose {
    for _, r := range vr {
      fmt.Printf("  - %s\n", r.Msg)
    }
  }
} else {
  fmt.Println("FAIL")
  // default, only print out failed validations
  for _, r := range vr {
    if !r.Pass {
      fmt.Printf("  - %s\n", r.Msg)
    }
  }
}

So keep the Printf, but drop the r.Pass check from the ‘fail== 0’
branch (because we know that none of the checks failed).

If you want to see passes in all cases, I think you want:

if _, fail := vr.PassFail(); fail == 0 {
  fmt.Println("PASS")
} else {
  fmt.Println("FAIL")
}
if *flVerbose {
  for _, r := range vr {
    if r.Pass {
      fmt.Printf("ok")
    }
    if r.Pass {
      fmt.Printf("not ok")
    }
    fmt.Println(" %s", r.Msg)
  }
}

Copy link
Member Author

@vbatts vbatts Sep 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not. test the code. ...

Copy link
Contributor

@wking wking Sep 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Thu, Sep 10, 2015 at 11:16:35AM -0700, Vincent Batts wrote:

  •               if !r.Pass {

  •               if r.Pass {

not. test the code. ...

I spun off my suggestion into #174.

fmt.Printf(" - %s\n", r.Msg)
}
}
Expand Down Expand Up @@ -127,51 +133,26 @@ func (vr ValidateResults) PassFail() (pass int, fail int) {
}

// CommitEntry represents a single commit's information from `git`
type CommitEntry struct {
Commit string
AbbreviatedCommit string `json:"abbreviated_commit"`
Tree string
AbbreviatedTree string `json:"abbreviated_tree"`
Parent string
AbbreviatedParent string `json:"abbreviated_parent"`
Refs string
Encoding string
Subject string
SanitizedSubjectLine string `json:"sanitized_subject_line"`
Body string
CommitNotes string `json:"commit_notes"`
VerificationFlag string `json:"verification_flag"`
ShortMsg string
Signer string
SignerKey string `json:"signer_key"`
Author PersonAction `json:"author,omitempty"`
Committer PersonAction `json:"committer,omitempty"`
}

// PersonAction is a time and identity of an action on a git commit
type PersonAction struct {
Name string `json:"name"`
Email string `json:"email"`
Date string `json:"date"` // this could maybe be an actual time.Time
}
type CommitEntry map[string]string

var (
prettyLogSubject = `--pretty=format:%s`
prettyLogBody = `--pretty=format:%b`
prettyLogCommit = `--pretty=format:%H`
prettyLogAuthorName = `--pretty=format:%aN`
prettyLogAuthorEmail = `--pretty=format:%aE`
prettyLogCommitterName = `--pretty=format:%cN`
prettyLogCommitterEmail = `--pretty=format:%cE`
prettyLogSigner = `--pretty=format:%GS`
prettyLogCommitNotes = `--pretty=format:%N`
prettyLogFormat = `--pretty=format:{"commit": "%H", "abbreviated_commit": "%h", "tree": "%T", "abbreviated_tree": "%t", "parent": "%P", "abbreviated_parent": "%p", "refs": "%D", "encoding": "%e", "sanitized_subject_line": "%f", "verification_flag": "%G?", "signer_key": "%GK", "author": { "date": "%aD" }, "committer": { "date": "%cD" }}`
prettyFormat = `--pretty=format:`
formatSubject = `%s`
formatBody = `%b`
formatCommit = `%H`
formatAuthorName = `%aN`
formatAuthorEmail = `%aE`
formatCommitterName = `%cN`
formatCommitterEmail = `%cE`
formatSigner = `%GS`
formatCommitNotes = `%N`
formatMap = `{"commit": "%H", "abbreviated_commit": "%h", "tree": "%T", "abbreviated_tree": "%t", "parent": "%P", "abbreviated_parent": "%p", "refs": "%D", "encoding": "%e", "sanitized_subject_line": "%f", "verification_flag": "%G?", "signer_key": "%GK", "author_date": "%aD" , "committer_date": "%cD" }`
)

// GitLogCommit assembles the full information on a commit from its commit hash
func GitLogCommit(commit string) (*CommitEntry, error) {
buf := bytes.NewBuffer([]byte{})
cmd := exec.Command("git", "log", "-1", prettyLogFormat, commit)
cmd := exec.Command("git", "log", "-1", prettyFormat+formatMap, commit)
cmd.Stdout = buf
cmd.Stderr = os.Stderr

Expand All @@ -186,53 +167,23 @@ func GitLogCommit(commit string) (*CommitEntry, error) {
return nil, err
}

output, err := exec.Command("git", "log", "-1", prettyLogSubject, commit).Output()
if err != nil {
return nil, err
}
c.Subject = strings.TrimSpace(string(output))

output, err = exec.Command("git", "log", "-1", prettyLogBody, commit).Output()
if err != nil {
return nil, err
}
c.Body = strings.TrimSpace(string(output))

output, err = exec.Command("git", "log", "-1", prettyLogAuthorName, commit).Output()
if err != nil {
return nil, err
}
c.Author.Name = strings.TrimSpace(string(output))

output, err = exec.Command("git", "log", "-1", prettyLogAuthorEmail, commit).Output()
if err != nil {
return nil, err
}
c.Author.Email = strings.TrimSpace(string(output))

output, err = exec.Command("git", "log", "-1", prettyLogCommitterName, commit).Output()
if err != nil {
return nil, err
}
c.Committer.Name = strings.TrimSpace(string(output))

output, err = exec.Command("git", "log", "-1", prettyLogCommitterEmail, commit).Output()
if err != nil {
return nil, err
}
c.Committer.Email = strings.TrimSpace(string(output))

output, err = exec.Command("git", "log", "-1", prettyLogCommitNotes, commit).Output()
if err != nil {
return nil, err
}
c.CommitNotes = strings.TrimSpace(string(output))

output, err = exec.Command("git", "log", "-1", prettyLogSigner, commit).Output()
if err != nil {
return nil, err
// any user provided fields can't be sanitized for the mock-json marshal above
for k, v := range map[string]string{
"subject": formatSubject,
"body": formatBody,
"author_name": formatAuthorName,
"author_email": formatAuthorEmail,
"committer_name": formatCommitterName,
"committer_email": formatCommitterEmail,
"commit_notes": formatCommitNotes,
"signer": formatSigner,
} {
output, err := exec.Command("git", "log", "-1", prettyFormat+v, commit).Output()
if err != nil {
return nil, err
}
c[k] = strings.TrimSpace(string(output))
}
c.Signer = strings.TrimSpace(string(output))

return &c, nil
}
Expand All @@ -241,7 +192,7 @@ func GitLogCommit(commit string) (*CommitEntry, error) {
// If commitrange is a git still range 12345...54321, then it will be isolated set of commits.
// If commitrange is a single commit, all ancestor commits up through the hash provided.
func GitCommits(commitrange string) ([]CommitEntry, error) {
output, err := exec.Command("git", "log", prettyLogCommit, commitrange).Output()
output, err := exec.Command("git", "log", prettyFormat+formatCommit, commitrange).Output()
if err != nil {
return nil, err
}
Expand Down