Skip to content

Conversation

@vbatts
Copy link
Member

@vbatts vbatts commented Sep 10, 2015

merged PRs do not have DCO, so do not check for them

#169

Signed-off-by: Vincent Batts [email protected]

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of splitting at consume-time, it seems cleaner to make CommitEntry.Parent a CommitEntry.Parents array and parse at commit-lookup time.

Copy link
Member Author

Choose a reason for hiding this comment

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

meh. that would be another separated shell-out, and would then also have to be done for abbreviated parent as well.

Copy link
Contributor

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:26:27AM -0700, Vincent Batts wrote:

@@ -16,6 +16,12 @@ import (
var DefaultRules = []ValidateRule{
func(c CommitEntry) (vr ValidateResult) {
vr.CommitEntry = c

  •   if len(strings.Split(c.Parent, " ")) > 1 {
    

meh. that would be another separated shell-out…

Why? Can't you just have:

… "parents": ["%P"], "abbreviated_parents": ["%p"], …,

in prettyLogFormat, and then:

c.Parents = strings.Split(c.Parents, " ")
c.AbbreviatedParents = strings.Split(c.AbbreviatedParents, " ")

after your Unmarshal?

merged PRs do not have DCO, so do not check for them

Signed-off-by: Vincent Batts <[email protected]>
Accomplishing the same access to the commit entry, with more readable
code.

Reported-by: Lai Jiangshan <[email protected]>
Signed-off-by: Vincent Batts <[email protected]>
@vbatts vbatts force-pushed the fix-dco-validate-on-merges branch from 546d18a to 5a654b9 Compare September 10, 2015 14:47
@vbatts
Copy link
Member Author

vbatts commented Sep 10, 2015

@crosbymichael
Copy link
Member

LGTM

@vbatts
Copy link
Member Author

vbatts commented Sep 10, 2015

@mrunalp review?

@mrunalp
Copy link
Contributor

mrunalp commented Sep 10, 2015

@vbatts Looking

@mrunalp
Copy link
Contributor

mrunalp commented Sep 10, 2015

LGTM

mrunalp pushed a commit that referenced this pull request Sep 10, 2015
travis: fix DCO validation for merges
@mrunalp mrunalp merged commit c00555c into opencontainers:master Sep 10, 2015
@vbatts vbatts deleted the fix-dco-validate-on-merges branch September 10, 2015 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants