* update hook for git to check push commit message
@ 2010-01-28 20:00 Khem Raj
2010-01-28 21:52 ` Phil Blundell
2010-01-28 22:59 ` Tom Rini
0 siblings, 2 replies; 10+ messages in thread
From: Khem Raj @ 2010-01-28 20:00 UTC (permalink / raw)
To: openembedded-devel
[-- Attachment #1: Type: text/plain, Size: 221 bytes --]
Hi
Attached is a small hook for updates that are pushed into repo.
Right now it only checks the first line of the commit and expects
module: summary
I have not tested it at all
Comments ?
Thanks
-Khem
[-- Attachment #2: update.py --]
[-- Type: text/x-python, Size: 1127 bytes --]
#!/usr/bin/env python
# usage update.py <oldrev> <newrev>
# currently it only checks for first line of the commit message
# which should look like e.g
# uclibc: <pretty summary>
import sys,os
from subprocess import *
oldrev = sys.argv[1]
newrev = sys.argv[2]
user = os.getenv('USER')
def ErrorOut(message):
print "Please reformat the message per guidelines on http://wiki.openembedded.org/index.php/Commit_Policy"
print "A sample is at http://wiki.openembedded.org/index.php/Commit_log_example"
print "\nFaulty Commit Message:\n\n ", message
sys.exit(1)
p = os.popen("git rev-list %s...%s" %(oldrev, newrev))
missed_revs = p.read()
for r in missed_revs.split():
m1 = Popen(["git", "cat-file", "commit", r],stdout=PIPE)
m2 = Popen(["sed", "1,/^$/d"], stdin=m1.stdout, stdout=PIPE)
rev = m2.communicate()[0]
first_word = rev.split()[0];
if first_word[-1] is ':':
print "Your message is not formatted according to commit poilicy"
ErrorOut(rev)
summary = rev.split(1)
if summary is "":
print "Commit Message does not have summary line"
ErrorOut(rev)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: update hook for git to check push commit message
2010-01-28 20:00 update hook for git to check push commit message Khem Raj
@ 2010-01-28 21:52 ` Phil Blundell
2010-01-28 22:01 ` Frans Meulenbroeks
2010-01-29 1:30 ` Khem Raj
2010-01-28 22:59 ` Tom Rini
1 sibling, 2 replies; 10+ messages in thread
From: Phil Blundell @ 2010-01-28 21:52 UTC (permalink / raw)
To: openembedded-devel
On Thu, 2010-01-28 at 12:00 -0800, Khem Raj wrote:
> Attached is a small hook for updates that are pushed into repo.
> Right now it only checks the first line of the commit and expects
> module: summary
I'm not really in favour of applying the commit message policy quite
that harshly. The wiki page that you linked to doesn't really seem to
support this kind of check: the actual guidance it gives about checkin
messages is:
* Have a clear commit message (example):
- The first line of commit is a summary of the changes.
- The first line should start with the name of the recipe the change affects.
- The rest of the message should give more details on the change as appropriate.
- Mention the affected bug numbers if appropriate.
- Give credit where credit is due. If you commit someone else's work more or
less verbatim, you should use git commit --author $mail-of-author. If pulling
changes from somewhere like Poky or OpenMoko there is no problem with that
but mention where the changes came from.
- Include a Signed-off-by: line indicating the change has valid certificate
of origin as per the Linux kernel
... which is nowhere near as prescriptive as the rule that you seem to
be implementing in your script. Also, the wiki page has an explicit
statement that "[these] rules are not hard fast rules", which would
suggest that even such guidance as it does provide is not intended to be
taken too dogmatically.
Personally I think that the current situation of the TSC applying
pressure to those people who seem to be chronically unable to write
suitable checkin comments is a perfectly fine way of solving the
problem.
On a technical level, I think you could probably do without the sed
subprocess, and "summary = rev.split(1)" doesn't look quite right
either.
p.
p.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: update hook for git to check push commit message
2010-01-28 21:52 ` Phil Blundell
@ 2010-01-28 22:01 ` Frans Meulenbroeks
2010-01-29 0:26 ` Paul Menzel
2010-01-29 1:30 ` Khem Raj
1 sibling, 1 reply; 10+ messages in thread
From: Frans Meulenbroeks @ 2010-01-28 22:01 UTC (permalink / raw)
To: openembedded-devel
2010/1/28 Phil Blundell <philb@gnu.org>:
> On Thu, 2010-01-28 at 12:00 -0800, Khem Raj wrote:
>> Attached is a small hook for updates that are pushed into repo.
>> Right now it only checks the first line of the commit and expects
>> module: summary
>
> I'm not really in favour of applying the commit message policy quite
> that harshly. The wiki page that you linked to doesn't really seem to
> support this kind of check: the actual guidance it gives about checkin
> messages is:
>
> * Have a clear commit message (example):
> - The first line of commit is a summary of the changes.
> - The first line should start with the name of the recipe the change affects.
What if a commit is not about a recipe but e.g. about a class file
and what if a commit changes multiple recipes ? (i had this one once
when eliminating lots of perl native recipes replacing them with
BBCLASSEXTEND="native")
This was a pain editing already. I think I would have stopped doing it
if I had to commit each individual patch
> - The rest of the message should give more details on the change as appropriate.
> - Mention the affected bug numbers if appropriate.
> - Give credit where credit is due. If you commit someone else's work more or
> less verbatim, you should use git commit --author $mail-of-author. If pulling
> changes from somewhere like Poky or OpenMoko there is no problem with that
> but mention where the changes came from.
> - Include a Signed-off-by: line indicating the change has valid certificate
> of origin as per the Linux kernel
Not sure if we can influence the template that you get with git commit
but I would surely like to learn if one way or another I can add a
Signed-off-by lne automatically
>
> ... which is nowhere near as prescriptive as the rule that you seem to
> be implementing in your script. Also, the wiki page has an explicit
> statement that "[these] rules are not hard fast rules", which would
> suggest that even such guidance as it does provide is not intended to be
> taken too dogmatically.
>
> Personally I think that the current situation of the TSC applying
> pressure to those people who seem to be chronically unable to write
> suitable checkin comments is a perfectly fine way of solving the
> problem.
Agree
FM
>
> On a technical level, I think you could probably do without the sed
> subprocess, and "summary = rev.split(1)" doesn't look quite right
> either.
>
> p.
>
>
> p.
>
>
>
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: update hook for git to check push commit message
2010-01-28 20:00 update hook for git to check push commit message Khem Raj
2010-01-28 21:52 ` Phil Blundell
@ 2010-01-28 22:59 ` Tom Rini
2010-01-28 23:22 ` Chris Larson
1 sibling, 1 reply; 10+ messages in thread
From: Tom Rini @ 2010-01-28 22:59 UTC (permalink / raw)
To: openembedded-devel
On Thu, 2010-01-28 at 12:00 -0800, Khem Raj wrote:
> Hi
>
> Attached is a small hook for updates that are pushed into repo.
> Right now it only checks the first line of the commit and expects
> module: summary
Is there any way to force the commit to happen, even if the script
doesn't like it? Can the script edit the commit message, even? ie if
there's no git commit --ignore-prehook type option, could it see if the
first line is FORCE or something, edit that out and commit? This will
catch the poorly formed commit messages without being a big burden on
people doing things that don't fit well into the "module: summary" model
(that said, lib*-perl-native: Convert to BBCLASSEXTEND as a first line
makes sense to me).
--
Tom Rini <tom_rini@mentor.com>
Mentor Graphics Corporation
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: update hook for git to check push commit message
2010-01-28 22:59 ` Tom Rini
@ 2010-01-28 23:22 ` Chris Larson
2010-01-29 0:10 ` Tom Rini
2010-01-29 8:21 ` Jens Seidel
0 siblings, 2 replies; 10+ messages in thread
From: Chris Larson @ 2010-01-28 23:22 UTC (permalink / raw)
To: openembedded-devel
On Thu, Jan 28, 2010 at 3:59 PM, Tom Rini <tom_rini@mentor.com> wrote:
> On Thu, 2010-01-28 at 12:00 -0800, Khem Raj wrote:
> > Hi
> >
> > Attached is a small hook for updates that are pushed into repo.
> > Right now it only checks the first line of the commit and expects
> > module: summary
>
> Is there any way to force the commit to happen, even if the script
> doesn't like it? Can the script edit the commit message, even? ie if
> there's no git commit --ignore-prehook type option, could it see if the
> first line is FORCE or something, edit that out and commit? This will
> catch the poorly formed commit messages without being a big burden on
> people doing things that don't fit well into the "module: summary" model
> (that said, lib*-perl-native: Convert to BBCLASSEXTEND as a first line
> makes sense to me).
>
My worry with editing a commit at push time is, the user's local repository
now no longer matches upstream. One would hope a rebase would intelligently
handle it, but from a configuration management perspective I don't think
modifying commits behind the user's back is a very sane thing.
Now, if you did this locally at commit time rather than push time, I think
that would be less crazy, but unfortunately a clone can't bring along hooks
:\
--
Chris Larson
clarson at kergoth dot com
clarson at mvista dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Software Engineer
MontaVista Software, Inc.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: update hook for git to check push commit message
2010-01-28 23:22 ` Chris Larson
@ 2010-01-29 0:10 ` Tom Rini
2010-01-29 8:21 ` Jens Seidel
1 sibling, 0 replies; 10+ messages in thread
From: Tom Rini @ 2010-01-29 0:10 UTC (permalink / raw)
To: openembedded-devel
On Thu, 2010-01-28 at 16:22 -0700, Chris Larson wrote:
> On Thu, Jan 28, 2010 at 3:59 PM, Tom Rini <tom_rini@mentor.com> wrote:
>
> > On Thu, 2010-01-28 at 12:00 -0800, Khem Raj wrote:
> > > Hi
> > >
> > > Attached is a small hook for updates that are pushed into repo.
> > > Right now it only checks the first line of the commit and expects
> > > module: summary
> >
> > Is there any way to force the commit to happen, even if the script
> > doesn't like it? Can the script edit the commit message, even? ie if
> > there's no git commit --ignore-prehook type option, could it see if the
> > first line is FORCE or something, edit that out and commit? This will
> > catch the poorly formed commit messages without being a big burden on
> > people doing things that don't fit well into the "module: summary" model
> > (that said, lib*-perl-native: Convert to BBCLASSEXTEND as a first line
> > makes sense to me).
> >
>
> My worry with editing a commit at push time is, the user's local repository
> now no longer matches upstream. One would hope a rebase would intelligently
> handle it, but from a configuration management perspective I don't think
> modifying commits behind the user's back is a very sane thing.
>
> Now, if you did this locally at commit time rather than push time, I think
> that would be less crazy, but unfortunately a clone can't bring along hooks
> :\
Ah, I forgot it would be on the server side and I don't see a way to
force the push if a hook fails.
--
Tom Rini <tom_rini@mentor.com>
Mentor Graphics Corporation
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: update hook for git to check push commit message
2010-01-28 22:01 ` Frans Meulenbroeks
@ 2010-01-29 0:26 ` Paul Menzel
0 siblings, 0 replies; 10+ messages in thread
From: Paul Menzel @ 2010-01-29 0:26 UTC (permalink / raw)
To: openembedded-devel
[-- Attachment #1: Type: text/plain, Size: 488 bytes --]
Am Donnerstag, den 28.01.2010, 23:01 +0100 schrieb Frans Meulenbroeks:
> 2010/1/28 Phil Blundell <philb@gnu.org>:
[…]
> > - Include a Signed-off-by: line indicating the change has valid certificate
> > of origin as per the Linux kernel
>
> Not sure if we can influence the template that you get with git commit
> but I would surely like to learn if one way or another I can add a
> Signed-off-by lne automatically
$ git commit -s
[…]
Thanks,
Paul
[-- Attachment #2: Dies ist ein digital signierter Nachrichtenteil --]
[-- Type: application/pgp-signature, Size: 205 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: update hook for git to check push commit message
2010-01-28 21:52 ` Phil Blundell
2010-01-28 22:01 ` Frans Meulenbroeks
@ 2010-01-29 1:30 ` Khem Raj
1 sibling, 0 replies; 10+ messages in thread
From: Khem Raj @ 2010-01-29 1:30 UTC (permalink / raw)
To: openembedded-devel
On (28/01/10 21:52), Phil Blundell wrote:
> On Thu, 2010-01-28 at 12:00 -0800, Khem Raj wrote:
> > Attached is a small hook for updates that are pushed into repo.
> > Right now it only checks the first line of the commit and expects
> > module: summary
>
> I'm not really in favour of applying the commit message policy quite
> that harshly. The wiki page that you linked to doesn't really seem to
> support this kind of check: the actual guidance it gives about checkin
> messages is:
Thats true. However I have seen many devs complain about the module:
summary. That was my motivation.
>
> * Have a clear commit message (example):
> - The first line of commit is a summary of the changes.
> - The first line should start with the name of the recipe the change affects.
> - The rest of the message should give more details on the change as appropriate.
> - Mention the affected bug numbers if appropriate.
> - Give credit where credit is due. If you commit someone else's work more or
> less verbatim, you should use git commit --author $mail-of-author. If pulling
> changes from somewhere like Poky or OpenMoko there is no problem with that
> but mention where the changes came from.
> - Include a Signed-off-by: line indicating the change has valid certificate
> of origin as per the Linux kernel
>
> ... which is nowhere near as prescriptive as the rule that you seem to
> be implementing in your script. Also, the wiki page has an explicit
> statement that "[these] rules are not hard fast rules", which would
> suggest that even such guidance as it does provide is not intended to be
> taken too dogmatically.
>
> Personally I think that the current situation of the TSC applying
> pressure to those people who seem to be chronically unable to write
> suitable checkin comments is a perfectly fine way of solving the
> problem.
Probably it could help developers to check the commit messages on their own
by making this as post commit or pre push hook of some sort in their clones .git/hooks
>
> On a technical level, I think you could probably do without the sed
> subprocess, and "summary = rev.split(1)" doesn't look quite right
> either.
true. I could have got it if I have tested it. The check for : should also
be inverted.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: update hook for git to check push commit message
2010-01-28 23:22 ` Chris Larson
2010-01-29 0:10 ` Tom Rini
@ 2010-01-29 8:21 ` Jens Seidel
2010-01-29 9:15 ` Martin Jansa
1 sibling, 1 reply; 10+ messages in thread
From: Jens Seidel @ 2010-01-29 8:21 UTC (permalink / raw)
To: openembedded-devel
On Thu, Jan 28, 2010 at 04:22:32PM -0700, Chris Larson wrote:
> On Thu, Jan 28, 2010 at 3:59 PM, Tom Rini <tom_rini@mentor.com> wrote:
> > On Thu, 2010-01-28 at 12:00 -0800, Khem Raj wrote:
> > > Attached is a small hook for updates that are pushed into repo.
> >
> > Is there any way to force the commit to happen, even if the script
> > doesn't like it? Can the script edit the commit message, even? ie if
> > there's no git commit --ignore-prehook type option, could it see if the
> > first line is FORCE or something, edit that out and commit? This will
> > catch the poorly formed commit messages without being a big burden on
> > people doing things that don't fit well into the "module: summary" model
> > (that said, lib*-perl-native: Convert to BBCLASSEXTEND as a first line
> > makes sense to me).
>
> My worry with editing a commit at push time is, the user's local repository
> now no longer matches upstream.
Right, what about two repositories, one with a hook which denies some
commits and another one without such a hook. The second repository could be
synced with the first via a cronjob and users are always forced to commit
to the first repository except they know *exactly* what they are doing
(maybe by restricting access to a few people only) and want to bypass these
checks?
Jens
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: update hook for git to check push commit message
2010-01-29 8:21 ` Jens Seidel
@ 2010-01-29 9:15 ` Martin Jansa
0 siblings, 0 replies; 10+ messages in thread
From: Martin Jansa @ 2010-01-29 9:15 UTC (permalink / raw)
To: openembedded-devel
On Fri, Jan 29, 2010 at 09:21:38AM +0100, Jens Seidel wrote:
> On Thu, Jan 28, 2010 at 04:22:32PM -0700, Chris Larson wrote:
> > On Thu, Jan 28, 2010 at 3:59 PM, Tom Rini <tom_rini@mentor.com> wrote:
> > > On Thu, 2010-01-28 at 12:00 -0800, Khem Raj wrote:
> > > > Attached is a small hook for updates that are pushed into repo.
> > >
> > > Is there any way to force the commit to happen, even if the script
> > > doesn't like it? Can the script edit the commit message, even? ie if
> > > there's no git commit --ignore-prehook type option, could it see if the
> > > first line is FORCE or something, edit that out and commit? This will
> > > catch the poorly formed commit messages without being a big burden on
> > > people doing things that don't fit well into the "module: summary" model
> > > (that said, lib*-perl-native: Convert to BBCLASSEXTEND as a first line
> > > makes sense to me).
> >
> > My worry with editing a commit at push time is, the user's local repository
> > now no longer matches upstream.
>
> Right, what about two repositories, one with a hook which denies some
> commits and another one without such a hook. The second repository could be
> synced with the first via a cronjob and users are always forced to commit
> to the first repository except they know *exactly* what they are doing
> (maybe by restricting access to a few people only) and want to bypass these
> checks?
This looks like a bit overcomplicated sollution. I think that every
commit message "on edge" of commit policy can be made to go through.
ie:
fso_recipes: switch from SRCREV to SRCPV
instead of
fso recipes: switch from SRCREV to SRCPV
lib*-perl-native: Convert to BBCLASSEXTEND
could go through as long as it's splited by ' '
is almost as readable as original and easier to s/ /_/ then pusing to
different repo and waiting for cron job to sync it, hoping that nobody
else didn't commit conflicting change to right repo with right commit
message first.
Regards,
--
uin:136542059 jid:Martin.Jansa@gmail.com
Jansa Martin sip:jamasip@voip.wengo.fr
JaMa
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-01-29 9:18 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-28 20:00 update hook for git to check push commit message Khem Raj
2010-01-28 21:52 ` Phil Blundell
2010-01-28 22:01 ` Frans Meulenbroeks
2010-01-29 0:26 ` Paul Menzel
2010-01-29 1:30 ` Khem Raj
2010-01-28 22:59 ` Tom Rini
2010-01-28 23:22 ` Chris Larson
2010-01-29 0:10 ` Tom Rini
2010-01-29 8:21 ` Jens Seidel
2010-01-29 9:15 ` Martin Jansa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.