'Authors' check in run_tests.sh makes life harder for new contributers

[Cole Robinson]

SInce gerrit commits are now gated on nova tests passing, the unit test that checks for an up to date Authors file should really be dropped.

Consider the scenario I am currently in

- I've never committed to nova before so I am not in Authors
- I have 2 independent changes I want to submit to nova

My options are:

- Submit both changes independently, both updating Authors to appease the tests. If both are accepted, one gets in first and the other has a conflict, requiring me to resend.

- Submit one change as the parent of another. This is confusing to reviewers since the changes are orthogonal, and one change is blocked on the other being accepted/committed.

- Submit one change, sit on the other until the first is resolved, then submit the second fix.

That's all stop energy for not a lot of benefit that I can see. The preferred way would be:

- Submit both changes independently and not worry about updating Authors.

If yall wanted to get fancy there is probably some git hook that can be used to auto update Authors. Simpler would be to just have the release manager regenerate Authors as part of cutting a release.

[Thierry Carrez (ttx)]

I think Authors needs to be in sync with tree contents at any time, and not just for cosmetic reasons. Submitting one change as the parent of the other doesn't sound like too much of a hassle, to keep that benefit.

A git hook could replace it, but without .mailmap it can quickly turn the Authors file into something ugly.. and I'm not sure our Gerrit system would support that.

Adding openstack-ci for more comments.

[Cole Robinson (crobinso)]

I assume the implication is that there are legal reasons for this, otherwise really what _is_ the benefit over something like 'git log' (and even in the legal case why isn't 'git log' sufficient)?

[Dave Walker (davewalker)]

It's a valid point, the git log (with suitable --format) can create an accurate list of all Authors. What benefit does a .mailmap add? I agree the .mailmap shrinking will be lost.. but really.. in years to come, will we *really* enjoy having a list of 1000's of entries anyway?

[Monty Taylor (mordred)]

On 02/10/2012 03:18 PM, Dave Walker wrote:
> It's a valid point, the git log (with suitable --format) can create an
> accurate list of all Authors. What benefit does a .mailmap add? I
> agree the .mailmap shrinking will be lost.. but really.. in years to
> come, will we *really* enjoy having a list of 1000's of entries anyway?

Also, now that we've got Gerrit gating CLA signature, the git log is
going to be really accurate. I kind of feel like the Authors check is a
hold solution for a problem we don't have any more.

How about we remove the Authors file altogether and have setup.py sdist
generate it from git?

Monty

[Jay Pipes (jaypipes)]

Agreed with Monty here. If we can automate the creation of an Authors file (now that Gerrit is gating on CLA signature) I think that makes much more sense.

[Thierry Carrez (ttx)]

I suppose git log will create multiple entries for the same person using multiple email addresses in git commits... The .mailmap reduction allowed us to have a sane list. It's used for PTL election purposes, so it's good to have one entry = one person.

Or is there another way to automagically reduce the list to current email addresses ?

[Soren Hansen (soren)]

2012/2/20 Thierry Carrez <email address hidden>:
> I suppose git log will create multiple entries for the same person using
> multiple email addresses in git commits... The .mailmap reduction
> allowed us to have a sane list. It's used for PTL election purposes, so
> it's good to have one entry = one person.
>
> Or is there another way to automagically reduce the list to current
> email addresses ?

git log already respects .mailmap (that's where I got the idea when I
wrote the implementation for the check back when we used bzr), so it
should "Just Work[tm]".

[Thierry Carrez (ttx)]

So we could... keep .mailmap but autogenerate Authors ? Sounds like a good way to generate duplicate entries for extra addresses missing a .mailmap entry...

[Bhuvan Arumugam (bhuvan)]

Filed bug 976267 to track the change related to auto-generating AUTHORS file. When it's in place, we could remove the test test_authors.py, that check for an entry in AUTHORS file.

[Bhuvan Arumugam (bhuvan)]

Proposed a patch for review, to remove this test case. As per bug 976267, a patch is in queue to auto generate AUTHORS file.
  https://review.openstack.org/6701

For now, i've posted the patch for keystone.

I've posted patches for bug 976267 to auto generate AUTHORS file. Once those patches are merged, I'll propose patches to remove this test case, for other components.

[Bhuvan Arumugam (bhuvan)]

The code to auto generate AUTHORS file and remove the test case has been merged for glance component.
  https://review.openstack.org/#/c/6698/

[Bhuvan Arumugam (bhuvan)]

The test case is removed/merged from following projects:
  * keystone
  * glance
  * openstack-common
  * python-openstackclient

The patch for review has been posted for following projects:
  * nova https://review.openstack.org/#/c/6699/

I'm working on a patch to remove this test case for following projects:
  * quantum
  * horizon
  * swift

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/9369

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/9369
Committed: http://github.com/openstack/nova/commit/3456b667c38aa1d439482b9c95838aba6d3d6c5c
Submitter: Jenkins
Branch: master

commit 3456b667c38aa1d439482b9c95838aba6d3d6c5c
Author: Monty Taylor <email address hidden>
Date: Thu Jul 5 09:08:38 2012 -0500

    Finish AUTHORS transition.

    The code to generate the authors file from the git changelog has been in the
    tree and running for a few weeks now. Somehow the removal of the authors
    test and and the MANIFEST.in file were wrong though. This should clean that
    up.

    Fixes bug 920757.

    Change-Id: I66c388c1c81804f8dabc52b5ee25c7f394921e11

[Samuel Merritt (torgomatic)]

There's no AUTHORS check in Swift's tests, so I'm guessing it got removed at some point and this bug didn't get updated.

[John Dickinson (notmyname)]

Actually, there was never an authors check in swift. Same overall result, though.