6:05 a.m.
As a general rule most libvirt commits in recent history have have a
valid Signed-off-by line present. There are still cases, however, where
this is being forgotten. It is time to put some checks on git pushes to
detect missinged Signed-off-by lines and reject the push.
I created a new git repository that contains the original git repository
update hook Eric provided years back. This commit then augments that
hook to add the Signed-off-by check. The updated hook gets automatically
installed on the server when we push to this repo.
Daniel P. Berrange (1):
Add check for Signed-off-by in commit messages
update | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
--
2.14.3
6:05 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
This extends the update hook so that it enforces a requirement to have a
Signed-off-by line in every commit message. This can be optionally
turned off in individual repos by setting the "hooks.allowmissingsob"
git config variable.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
update | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/update b/update
index 6a8edcb..b7cfe7f 100755
--- a/update
+++ b/update
@@ -248,7 +248,21 @@ if [ $check_diff = yes ]; then
if [ "$allow_bad_whitespace" != "true" ]; then
test "$oldrev" = $zero \
&& exit 0
- exec git diff --check $oldrev $newrev --
+ git diff --check $oldrev $newrev --
+ test $? != 0 && exit 1
+ fi
+
+ allow_missing_sob=$(git config --bool hooks.allowmissingsob)
+ if [ "$allow_missing_sob" != "true" ]; then
+ for rev in `git log --format=%h $oldrev..$newrev`
+ do
+ git show $rev | grep Signed-off-by >/dev/null 2>&1
+ if test $? != 0
+ then
+ echo "*** Update hook: missing Signed-off-by tag in $rev" >&2
+ exit 1
+ fi
+ done
fi
fi
--
2.14.3
6:20 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
On Mon, Jan 22, 2018 at 12:05:19 +0000, Daniel Berrange wrote:
This extends the update hook so that it enforces a requirement to
have a
Signed-off-by line in every commit message. This can be optionally
turned off in individual repos by setting the "hooks.allowmissingsob"
git config variable.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
update | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
NACK, I don't like signoffs and I don't really think they achieve
anything.
7:06 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
On Mon, Jan 22, 2018 at 01:20:12PM +0100, Peter Krempa wrote:
On Mon, Jan 22, 2018 at 12:05:19 +0000, Daniel Berrange wrote:
> This extends the update hook so that it enforces a requirement to have a
> Signed-off-by line in every commit message. This can be optionally
> turned off in individual repos by setting the "hooks.allowmissingsob"
> git config variable.
>
> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> ---
> update | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
NACK, I don't like signoffs and I don't really think they achieve
anything.
A signoff with no documented meaning attached by the project is fairly
weak, as you would have to argue there was some commonly accepted signifance
to it across the community. A signoff which is explicitly associated with a
statement of intent has benefit, hence why I also sent a patch to clarify
that the signoff asserts compliance with the DCO. Adding these signoffs has
little to no time burden on long term developers own work on a daily basis.
Just needs adding -s to "git commit" which quickly becomes engrained in
muscle memory such that you'll end up doing it for every project you find
yourself contributing to. Many of our "drive by" contributors already do
this as habit, we'll just need to remind those that forget periodically.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
7:20 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
On Mon, Jan 22, 2018 at 13:06:28 +0000, Daniel Berrange wrote:
On Mon, Jan 22, 2018 at 01:20:12PM +0100, Peter Krempa wrote:
> On Mon, Jan 22, 2018 at 12:05:19 +0000, Daniel Berrange wrote:
> > This extends the update hook so that it enforces a requirement to have a
> > Signed-off-by line in every commit message. This can be optionally
> > turned off in individual repos by setting the
"hooks.allowmissingsob"
> > git config variable.
> >
> > Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> > ---
> > update | 16 +++++++++++++++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
>
> NACK, I don't like signoffs and I don't really think they achieve
> anything.
A signoff with no documented meaning attached by the project is fairly
weak, as you would have to argue there was some commonly accepted signifance
to it across the community. A signoff which is explicitly associated with a
statement of intent has benefit, hence why I also sent a patch to clarify
that the signoff asserts compliance with the DCO. Adding these signoffs has
You can do the same by declaring that all patches have to comply to that
and not mandating adding a line which will become eventually pointless
since every patch will need to have it.
Also since there's no way to check that it's actually true, anybody can
declare anything. Also the check itself can be fooled easily, so I think
it's pointless altogether.
little to no time burden on long term developers own work on a daily
basis.
Just needs adding -s to "git commit" which quickly becomes engrained in
muscle memory such that you'll end up doing it for every project you find
Or you can defeat it entirely by adding it into your commit message
template. I don't care about the added burden though. I don't really
think it achieves anything.
yourself contributing to. Many of our "drive by"
contributors already do
this as habit, we'll just need to remind those that forget periodically.
I presonally signed-off < 5 commits in libvirt. So the last statement
is untrue. Some people don't do it on purpose.
The sign-off by itself (whithout cryptographic signature) is just
pointless. Validity with a cryptographic signature from drive-by
contributors can still be unproven, but at least you don't get
impersonation.
If everything is signed off, nothing really is.
NACK still stands.
7:57 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
On Mon, Jan 22, 2018 at 02:20:52PM +0100, Peter Krempa wrote:
On Mon, Jan 22, 2018 at 13:06:28 +0000, Daniel Berrange wrote:
> On Mon, Jan 22, 2018 at 01:20:12PM +0100, Peter Krempa wrote:
> > On Mon, Jan 22, 2018 at 12:05:19 +0000, Daniel Berrange wrote:
> > > This extends the update hook so that it enforces a requirement to have a
> > > Signed-off-by line in every commit message. This can be optionally
> > > turned off in individual repos by setting the
"hooks.allowmissingsob"
> > > git config variable.
> > >
> > > Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> > > ---
> > > update | 16 +++++++++++++++-
> > > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > NACK, I don't like signoffs and I don't really think they achieve
> > anything.
>
> A signoff with no documented meaning attached by the project is fairly
> weak, as you would have to argue there was some commonly accepted signifance
> to it across the community. A signoff which is explicitly associated with a
> statement of intent has benefit, hence why I also sent a patch to clarify
> that the signoff asserts compliance with the DCO. Adding these signoffs has
You can do the same by declaring that all patches have to comply to that
and not mandating adding a line which will become eventually pointless
since every patch will need to have it.
Also since there's no way to check that it's actually true, anybody can
declare anything. Also the check itself can be fooled easily, so I think
it's pointless altogether.
The git hook isn't expected to be flawless. Whomever is pushing the patches
to git is still actually reviewing the submissions. If someone puts garbage
there it will still likely be caught just as if someone puts garbage in the
actual code.
> little to no time burden on long term developers own work on a
daily basis.
> Just needs adding -s to "git commit" which quickly becomes engrained in
> muscle memory such that you'll end up doing it for every project you find
Or you can defeat it entirely by adding it into your commit message
template. I don't care about the added burden though. I don't really
think it achieves anything.
So we can weigh up the possible outcomes. It it adopt it and it turns
out to not do anything, then we've merely suffered insigifincant burden
of adding it. If we don't adopt it, and it does turn out to be important
the consequences may we quite severe. It makes no prudent sense to
not adopt it, given the minimal burden it has and the potentially significant
upside.
> yourself contributing to. Many of our "drive by"
contributors already do
> this as habit, we'll just need to remind those that forget periodically.
I presonally signed-off < 5 commits in libvirt. So the last statement
is untrue. Some people don't do it on purpose.
The sign-off by itself (whithout cryptographic signature) is just
pointless. Validity with a cryptographic signature from drive-by
contributors can still be unproven, but at least you don't get
impersonation.
I think these are two different axis. The sob isn't trying to address the
question of impersonation. It obviously has as a starting point that you
accept the identity of the submitter to some degree. I accept that if you
have cryptographically signed patches, that would give a stronger
validation of identity, but there's never any absolutes. So not having
a crypto signature doesn't make the sob invalid.
If everything is signed off, nothing really is.
I don't really see that.
NACK still stands.
You are nacking something that you've accepted above will have no negative
impact on your work, but has potentially significant upside to the project.
That is very disappointing.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
4:13 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
On Mon, Jan 22, 2018 at 13:57:07 +0000, Daniel Berrange wrote:
On Mon, Jan 22, 2018 at 02:20:52PM +0100, Peter Krempa wrote:
> On Mon, Jan 22, 2018 at 13:06:28 +0000, Daniel Berrange wrote:
> > On Mon, Jan 22, 2018 at 01:20:12PM +0100, Peter Krempa wrote:
> > > On Mon, Jan 22, 2018 at 12:05:19 +0000, Daniel Berrange wrote:
> > > > This extends the update hook so that it enforces a requirement to
have a
> > > > Signed-off-by line in every commit message. This can be optionally
> > > > turned off in individual repos by setting the
"hooks.allowmissingsob"
> > > > git config variable.
> > > >
> > > > Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> > > > ---
> > > > update | 16 +++++++++++++++-
> > > > 1 file changed, 15 insertions(+), 1 deletion(-)
[..]
> The sign-off by itself (whithout cryptographic signature) is
just
> pointless. Validity with a cryptographic signature from drive-by
> contributors can still be unproven, but at least you don't get
> impersonation.
I think these are two different axis. The sob isn't trying to address the
question of impersonation. It obviously has as a starting point that you
accept the identity of the submitter to some degree. I accept that if you
have cryptographically signed patches, that would give a stronger
validation of identity, but there's never any absolutes. So not having
a crypto signature doesn't make the sob invalid.
In that case basically nothing changes, since if we are going to use
this to be safe from licensing disputes, the reviewer/commiter still
needs to make sure that the code complies with our licensing. Asserting
the signoff changes nothing in that regard
> If everything is signed off, nothing really is.
I don't really see that.
> NACK still stands.
You are nacking something that you've accepted above will have no negative
impact on your work, but has potentially significant upside to the project.
That is very disappointing.
I think that by doing this we'll put too much false hope into the
"potentially significant upside". I just hope it will not bite us.
Anyone can assert, or sign-off anything [1].
Given the overwhelmingly positive approach to this retract my NACK, the
only thing that will change in general is that my commits will grow one
line.
I hope that I'm wrong with my pessimistic view.
Peter
[1] https://en.wikipedia.org/wiki/On_the_Internet,_nobody_knows_you're_a_dog
10:33 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
On Thu, Jan 25, 2018 at 11:13:40AM +0100, Peter Krempa wrote:
On Mon, Jan 22, 2018 at 13:57:07 +0000, Daniel Berrange wrote:
> On Mon, Jan 22, 2018 at 02:20:52PM +0100, Peter Krempa wrote:
> > On Mon, Jan 22, 2018 at 13:06:28 +0000, Daniel Berrange wrote:
> > > On Mon, Jan 22, 2018 at 01:20:12PM +0100, Peter Krempa wrote:
> > > > On Mon, Jan 22, 2018 at 12:05:19 +0000, Daniel Berrange wrote:
> > > > > This extends the update hook so that it enforces a requirement
to have a
> > > > > Signed-off-by line in every commit message. This can be
optionally
> > > > > turned off in individual repos by setting the
"hooks.allowmissingsob"
> > > > > git config variable.
> > > > >
> > > > > Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> > > > > ---
> > > > > update | 16 +++++++++++++++-
> > > > > 1 file changed, 15 insertions(+), 1 deletion(-)
[..]
> > The sign-off by itself (whithout cryptographic signature) is just
> > pointless. Validity with a cryptographic signature from drive-by
> > contributors can still be unproven, but at least you don't get
> > impersonation.
>
> I think these are two different axis. The sob isn't trying to address the
> question of impersonation. It obviously has as a starting point that you
> accept the identity of the submitter to some degree. I accept that if you
> have cryptographically signed patches, that would give a stronger
> validation of identity, but there's never any absolutes. So not having
> a crypto signature doesn't make the sob invalid.
In that case basically nothing changes, since if we are going to use
this to be safe from licensing disputes, the reviewer/commiter still
needs to make sure that the code complies with our licensing. Asserting
the signoff changes nothing in that regard
>
> > If everything is signed off, nothing really is.
>
> I don't really see that.
>
> > NACK still stands.
>
> You are nacking something that you've accepted above will have no negative
> impact on your work, but has potentially significant upside to the project.
> That is very disappointing.
I think that by doing this we'll put too much false hope into the
"potentially significant upside". I just hope it will not bite us.
Anyone can assert, or sign-off anything [1].
Given the overwhelmingly positive approach to this retract my NACK, the
only thing that will change in general is that my commits will grow one
line.
Thanks, I appreciate that.
FYI, i'm not going to push the hook right now, since I'm on holiday for
3 days and its always bad to do changes to dev workflow before going
away :-) I'll do it next week when i return....
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
8:11 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
On 01/22/2018 02:20 PM, Peter Krempa wrote:
On Mon, Jan 22, 2018 at 13:06:28 +0000, Daniel Berrange wrote:
> On Mon, Jan 22, 2018 at 01:20:12PM +0100, Peter Krempa wrote:
>> On Mon, Jan 22, 2018 at 12:05:19 +0000, Daniel Berrange wrote:
>>> This extends the update hook so that it enforces a requirement to have a
>>> Signed-off-by line in every commit message. This can be optionally
>>> turned off in individual repos by setting the
"hooks.allowmissingsob"
>>> git config variable.
>>>
>>> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
>>> ---
>>> update | 16 +++++++++++++++-
>>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> NACK, I don't like signoffs and I don't really think they achieve
>> anything.
>
> A signoff with no documented meaning attached by the project is fairly
> weak, as you would have to argue there was some commonly accepted signifance
> to it across the community. A signoff which is explicitly associated with a
> statement of intent has benefit, hence why I also sent a patch to clarify
> that the signoff asserts compliance with the DCO. Adding these signoffs has
You can do the same by declaring that all patches have to comply to that
and not mandating adding a line which will become eventually pointless
since every patch will need to have it.
Also since there's no way to check that it's actually true, anybody can
declare anything.
Sure, but then the burden is on you because you declared that you comply
with something by including that line while you cannot. I am no lawyer,
but if it can help us be stronger against law suits then so be it.
Also the check itself can be fooled easily, so I think
it's pointless altogether.
> little to no time burden on long term developers own work on a daily basis.
> Just needs adding -s to "git commit" which quickly becomes engrained in
> muscle memory such that you'll end up doing it for every project you find
Or you can defeat it entirely by adding it into your commit message
template. I don't care about the added burden though. I don't really
think it achieves anything.
> yourself contributing to. Many of our "drive by" contributors already do
> this as habit, we'll just need to remind those that forget periodically.
I presonally signed-off < 5 commits in libvirt. So the last statement
is untrue. Some people don't do it on purpose.
The sign-off by itself (whithout cryptographic signature) is just
pointless. Validity with a cryptographic signature from drive-by
contributors can still be unproven, but at least you don't get
impersonation.
If everything is signed off, nothing really is.
Not true. I view DCO as an agreement between Libvirt entity (whomever
represented by) and individual contributor that they are allowed by they
employer (or copyright holder) to send the patch. And thus, if the
agreement is signed more than once, it doesn't matter (although,
employees, occupancies, company cultures change through time). It's
still valid. But if it is not signed at all that's the problem.
Also, we will surely never experience this at Red Hat, but I've heard
many urban legends when an employee was not allowed to send a patch to
the upstream (because of some corporate politics or whatever). And I
think those are the cases we should protect us from.
Michal
6:22 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
On Mon, Jan 22, 2018 at 12:05:19PM +0000, Daniel P. Berrange wrote:
This extends the update hook so that it enforces a requirement to have
a
Signed-off-by line in every commit message. This can be optionally
turned off in individual repos by setting the "hooks.allowmissingsob"
git config variable.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
update | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
Given that signed-off-by lines are pointless for patches authored and
committed by the same person,
NACK unless the hooks.allowmissingsob will be set in the main
libvirt.git (I don't really care about other repos).
Jan
diff --git a/update b/update
index 6a8edcb..b7cfe7f 100755
--- a/update
+++ b/update
@@ -248,7 +248,21 @@ if [ $check_diff = yes ]; then
if [ "$allow_bad_whitespace" != "true" ]; then
test "$oldrev" = $zero \
&& exit 0
- exec git diff --check $oldrev $newrev --
+ git diff --check $oldrev $newrev --
+ test $? != 0 && exit 1
+ fi
+
+ allow_missing_sob=$(git config --bool hooks.allowmissingsob)
+ if [ "$allow_missing_sob" != "true" ]; then
+ for rev in `git log --format=%h $oldrev..$newrev`
+ do
+ git show $rev | grep Signed-off-by >/dev/null 2>&1
+ if test $? != 0
+ then
+ echo "*** Update hook: missing Signed-off-by tag in $rev" >&2
+ exit 1
+ fi
+ done
fi
fi
--
2.14.3
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
6:42 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
On Mon, Jan 22, 2018 at 01:22:01PM +0100, Ján Tomko wrote:
On Mon, Jan 22, 2018 at 12:05:19PM +0000, Daniel P. Berrange wrote:
> This extends the update hook so that it enforces a requirement to have a
> Signed-off-by line in every commit message. This can be optionally
> turned off in individual repos by setting the "hooks.allowmissingsob"
> git config variable.
>
> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> ---
> update | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
Given that signed-off-by lines are pointless for patches authored and
committed by the same person,
They are not pointless. They provide an explicit assertion that the author
is acknowledged they are permitted to make the contribution under the project's
license. This is distinct from the Author/Committer info in the commit, because
that is added automatically my git with no thought required by the developer.
NACK unless the hooks.allowmissingsob will be set in the main
libvirt.git (I don't really care about other repos).
I'm intending it to be set in *every* repository include libvirt.git. There
is no real world burden for developers to add a signed-off-by line to the
commits they contribute to the project, and it puts us in a stronger legal
position going forward. It is already commonplace across countless open
source projects, including many that we interact & build on in libvirt.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
7:24 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
On Mon, Jan 22, 2018 at 12:42:47PM +0000, Daniel P. Berrange wrote:
On Mon, Jan 22, 2018 at 01:22:01PM +0100, Ján Tomko wrote:
> On Mon, Jan 22, 2018 at 12:05:19PM +0000, Daniel P. Berrange wrote:
> > This extends the update hook so that it enforces a requirement to have a
> > Signed-off-by line in every commit message. This can be optionally
> > turned off in individual repos by setting the "hooks.allowmissingsob"
> > git config variable.
> >
> > Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> > ---
> > update | 16 +++++++++++++++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
>
> Given that signed-off-by lines are pointless for patches authored and
> committed by the same person,
They are not pointless. They provide an explicit assertion that the author
is acknowledged they are permitted to make the contribution under the project's
license. This is distinct from the Author/Committer info in the commit, because
that is added automatically my git with no thought required by the developer.
I refuse to believe that a group of programmers is incapable of automating such
mundane process.
Also, adding -s to the command line by muscle memory is by definition a
no-thought process.
> NACK unless the hooks.allowmissingsob will be set in the main
> libvirt.git (I don't really care about other repos).
I'm intending it to be set in *every* repository include libvirt.git. There
is no real world burden for developers to add a signed-off-by line to the
commits they contribute to the project,
It adds visual clutter and one unnecessary step.
and it puts us in a stronger legal
position going forward. It is already commonplace across countless open
source projects, including many that we interact & build on in libvirt.
Yes, and the amount of red tape required to contribute is off-putting.
It saddens me to see libvirt go that route.
Jan
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
7:41 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
On Mon, Jan 22, 2018 at 02:24:40PM +0100, Ján Tomko wrote:
On Mon, Jan 22, 2018 at 12:42:47PM +0000, Daniel P. Berrange wrote:
> On Mon, Jan 22, 2018 at 01:22:01PM +0100, Ján Tomko wrote:
> > On Mon, Jan 22, 2018 at 12:05:19PM +0000, Daniel P. Berrange wrote:
> > > This extends the update hook so that it enforces a requirement to have a
> > > Signed-off-by line in every commit message. This can be optionally
> > > turned off in individual repos by setting the
"hooks.allowmissingsob"
> > > git config variable.
> > >
> > > Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> > > ---
> > > update | 16 +++++++++++++++-
> > > 1 file changed, 15 insertions(+), 1 deletion(-)
> > >
> >
> > Given that signed-off-by lines are pointless for patches authored and
> > committed by the same person,
>
> They are not pointless. They provide an explicit assertion that the author
> is acknowledged they are permitted to make the contribution under the project's
> license. This is distinct from the Author/Committer info in the commit, because
> that is added automatically my git with no thought required by the developer.
>
I refuse to believe that a group of programmers is incapable of automating such
mundane process.
Also, adding -s to the command line by muscle memory is by definition a
no-thought process.
Those who are regular contributors to open source with sob understand what
it means. The infrequent or first time contributors would be those who are
making an explicit thought aobut it.
> > NACK unless the hooks.allowmissingsob will be set in the
main
> > libvirt.git (I don't really care about other repos).
>
> I'm intending it to be set in *every* repository include libvirt.git. There
> is no real world burden for developers to add a signed-off-by line to the
> commits they contribute to the project,
It adds visual clutter and one unnecessary step.
There's no real extra step here - its just a tweak to the existing commit
step, so doesn't add any burden. The "visual clutter" is not a relevant
point imho, not least because it is already widely present across our
commits.
> and it puts us in a stronger legal
> position going forward. It is already commonplace across countless open
> source projects, including many that we interact & build on in libvirt.
>
Yes, and the amount of red tape required to contribute is off-putting.
It saddens me to see libvirt go that route.
I think this is really overstating the impact of the change. Compared to the
extensive code style guidelines that users must follow when writing code,
this change will have no measurable negative impact on work involved in
contributing to libvirt.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
7:58 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
On 01/22/2018 07:05 AM, Daniel P. Berrange wrote:
This extends the update hook so that it enforces a requirement to
have a
Signed-off-by line in every commit message. This can be optionally
turned off in individual repos by setting the "hooks.allowmissingsob"
git config variable.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
update | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
Count me in as in favor of this. I have only ever had to add:
[format]
signoff = true
to my $HOME/.gitconfig once and it's been in place ever since. This is
far less than the:
[user] and [sendemail] sections and the same amount of additions to/for
the [push] section. One extra line.
It's also less than anything I've had to add to my repository specific
.git/config files.
It's also easier than the :
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
line that I keep in a "cheats" file that I always have open so that I
don't have to add it for patches I review. Of course, I could also just
say ACK, but the R-b seems so much more authoritative. Now if I could
only remember or figure out a way to add it to any patches I push
without having to remember to go back and rebase --interactive to add it
(which I rarely ever do).
John
1:30 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
On Mon, Jan 22, 2018 at 08:58:52AM -0500, John Ferlan wrote:
On 01/22/2018 07:05 AM, Daniel P. Berrange wrote:
> This extends the update hook so that it enforces a requirement to have a
> Signed-off-by line in every commit message. This can be optionally
> turned off in individual repos by setting the "hooks.allowmissingsob"
> git config variable.
>
> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> ---
> update | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
I've been using it since there was the first mention of it on the list and
because it didn't add any burden to my daily work and couldn't honestly be
simpler to configure, I don't have any objections to this move whatsoever.
Count me in as in favor of this. I have only ever had to add:
[format]
signoff = true
to my $HOME/.gitconfig once and it's been in place ever since. This is
far less than the:
[user] and [sendemail] sections and the same amount of additions to/for
the [push] section. One extra line.
It's also less than anything I've had to add to my repository specific
.git/config files.
It's also easier than the :
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
line that I keep in a "cheats" file that I always have open so that I
don't have to add it for patches I review. Of course, I could also just
say ACK, but the R-b seems so much more authoritative. Now if I could
only remember or figure out a way to add it to any patches I push
without having to remember to go back and rebase --interactive to add it
You're looking for git interpret-trailers...Ironically, I haven't been using it
even though I've known about it for quite some while, oh sigh, now's the right
time to do it.
Erik
3:35 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
On Mon, Jan 22, 2018 at 08:58:52AM -0500, John Ferlan wrote:
It's also easier than the :
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
line that I keep in a "cheats" file that I always have open so that I
don't have to add it for patches I review. Of course, I could also just
say ACK, but the R-b seems so much more authoritative.
FWIW, in emacs I have some macros that make it quick to type the long
statements, since it is so commonly needed in QEMU
(define-abbrev-table 'global-abbrev-table
'(
("8rev" "Reviewed-by: Daniel P. Berrange
<berrange(a)redhat.com>" nil 1)
("8ack" "Acked-by: Daniel P. Berrange <berrange(a)redhat.com>"
nil 1)
("8test" "Tested-by: Daniel P. Berrange
<berrange(a)redhat.com>" nil 1)
))
With that defined, if I type the word '8rev' and hit enter/spacebar then
it gets auto-expanded - so just as quick as typing 'ACK'.
Now if I
could
only remember or figure out a way to add it to any patches I push
without having to remember to go back and rebase --interactive to add it
(which I rarely ever do).
For sob it is easy you can automate with a non-interactive rebase
$ git rebase -i master -x "git commit --amend --no-edit -s"
For adding R-b/ACK/etc you could set EDITOR=/some/script which appends
them and to the same trick.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
10:24 a.m.
New subject: [libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages
On Mon, Jan 22, 2018 at 12:05:19PM +0000, Daniel P. Berrange wrote:
This extends the update hook so that it enforces a requirement to
have a
Signed-off-by line in every commit message. This can be optionally
turned off in individual repos by setting the "hooks.allowmissingsob"
git config variable.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
FWIW, as one of the drive-by people:
Reviewed-by: Kashyap Chamarthy <kchamart(a)redhat.com>
This change is simply making it easier to comply with DCO[*].
[*] https://developercertificate.org/
---
update | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/update b/update
index 6a8edcb..b7cfe7f 100755
--- a/update
+++ b/update
@@ -248,7 +248,21 @@ if [ $check_diff = yes ]; then
if [ "$allow_bad_whitespace" != "true" ]; then
test "$oldrev" = $zero \
&& exit 0
- exec git diff --check $oldrev $newrev --
+ git diff --check $oldrev $newrev --
+ test $? != 0 && exit 1
+ fi
+
+ allow_missing_sob=$(git config --bool hooks.allowmissingsob)
+ if [ "$allow_missing_sob" != "true" ]; then
+ for rev in `git log --format=%h $oldrev..$newrev`
+ do
+ git show $rev | grep Signed-off-by >/dev/null 2>&1
+ if test $? != 0
+ then
+ echo "*** Update hook: missing Signed-off-by tag in $rev" >&2
+ exit 1
+ fi
+ done
fi
fi
--
2.14.3
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
/kashyap
2493
days inactive
2496
days old
16 comments
8 participants
participants (8)
-
Daniel P. Berrange -
Daniel P. Berrangé
-
Erik Skultety -
John Ferlan -
Ján Tomko -
Kashyap Chamarthy -
Michal Privoznik -
Peter Krempa