.tools: repo validation tool

[vbatts]

Initially only a DCO validation for travis

#90

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

[wking]

We should probably either drop this debugging comment or uncomment it and put it behind a flag before landing this PR.

[vbatts]

yay. updated, and now that travis is enabled you can see that it also validated the DCO in this PR. :-)

[vbatts]

@tianon review?

[tianon]

Adorbs. 👍

[tianon]

(LGTM)

[wking]

If we never return anything other than nil for err, I'd just make the signature:

func(c CommitEntry) (vr ValidateResult)

[vbatts]

fair...

[wking]

Does Go not have an equivalent of Python's setattr? In Python, I'd make this:

for format, keys in [
        ('%s', ['subject']),
        …
        ('%cE', ['committer', 'email']),
        ]:
    value = …
    obj = commit
    for key in keys[:-1]:
        obj = getattr(obj, key)
    setattr(obj, keys[-1], value)

[vbatts]

Not without reflect, which is overkill for doing a simple loop.
On Sep 9, 2015 6:02 PM, "W. Trevor King" [email protected] wrote:

In .tools/validate.go
#167 (comment):

• if err != nil {

•   return nil, err
  

• }
• c.Author.Email = strings.TrimSpace(string(output))

• output, err = exec.Command("git", "log", "-1", prettyLogCommiterName, commit).Output()
• if err != nil {

•   return nil, err
  

• }
• c.Commiter.Name = strings.TrimSpace(string(output))

• output, err = exec.Command("git", "log", "-1", prettyLogCommiterEmail, commit).Output()
• if err != nil {

•   return nil, err
  

• }
• c.Commiter.Email = strings.TrimSpace(string(output))

Does Go not have an equivalent of Python's setattr? In Python, I'd make
this:

for format, keys in [
('%s', ['subject']),
…
('%cE', ['committer', 'email']),
]:
value = …
obj = commit
for key in keys[:-1]:
obj = getattr(obj, key)
setattr(obj, keys[-1], value)

—
Reply to this email directly or view it on GitHub
https://github.com/opencontainers/specs/pull/167/files#r39102627.

[wking]

On Wed, Sep 09, 2015 at 03:08:43PM -0700, Vincent Batts wrote:

Not without reflect, which is overkill for doing a simple loop.

Ah bummer. And I have a friend who would be waving his Lisp-y
parenthesis at this too ;).

[laijs]

Using the map instead of the structure can avoid reflect. Even using reflect, it is much simpler than current code IMO.

[vbatts]

updated ... PTAL!

[wking]

GitHub seems to have eaten my emailed response, so I'll repeat it here:

On Wed, Sep 09, 2015 at 03:15:53PM -0700, Vincent Batts wrote:

updated ... PTAL!

8b55acf looks good enough to me. I'd still recommend the --format
updates suggested in 1, but if you'd rather maintain it the way it
is, then I'm happy enough ;).

[vbatts]

@wking i'm not going to worry about depending on their git version.

[philips]

nit: not a huge fan of hiding code. tools without the .tools would be better IMHO

[philips]

good start, better than the nothing we have now. LGTM for merge.

[vbatts]

@philips i only hid it because it's like project housekeeping. If it were in ./tools/ or just ignored but sitting with a name like validate.go it may be expected for specs tools or validation. right?

[vbatts]

we can move it out later, or just have that kind of git validation be go getable for travis use. For now, I'm going to merge this for DCO enforcement of PRs

[vbatts]

lol. feature needed!

[wking]

On Wed, Sep 09, 2015 at 08:17:27PM -0700, Vincent Batts wrote:

we can move it out later, or just have that kind of git validation
be go getable for travis use…

This sounds good to me, especially since “I want to validate PRs
against our project's commit policy” seems like something that other
repositories might want to use too.

[vbatts]

Yup
On Sep 9, 2015 23:39, "W. Trevor King" [email protected] wrote:

On Wed, Sep 09, 2015 at 08:17:27PM -0700, Vincent Batts wrote:

we can move it out later, or just have that kind of git validation
be go getable for travis use…

This sounds good to me, especially since “I want to validate PRs
against our project's commit policy” seems like something that other
repositories might want to use too.

—
Reply to this email directly or view it on GitHub
#167 (comment).