1:06 p.m.
Repo: git://git.baserock.org/baserock/baserock/morph.git
Ref: baserock/richardmaw/style-checker
SHA1: 72a420f09835a31eb1ec097f2791a7603120a2d9
Land: origin/master
Currently the copyright year check lets things fall through the cracks
if ./check is run at the wrong time.
Previously, if you had added changes to a file, forgotten to change
the copyright year, and run ./check before committing, it would pass,
since it reads the copyright year from the commit.
The first patch fixes this, by checking `git status` to see if the file is
different to that which is committed, and if it is, use the current year.
You would also miss files that have never been committed, since `git
ls-files` doesn't show them by default; the second commit fixes this.
Richard Maw (2):
check-copyright-year: Use current date for uncommitted changes
style check: Include newly added files in checks
check | 5 +++--
scripts/check-copyright-year | 23 ++++++++++++++++++++---
2 files changed, 23 insertions(+), 5 deletions(-)
--
1.8.5.rc2
1:06 p.m.
New subject: [PATCH 1/2] check-copyright-year: Use current date for uncommitted changes
---
scripts/check-copyright-year | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/scripts/check-copyright-year b/scripts/check-copyright-year
index 99a6df9..d72ddbc 100755
--- a/scripts/check-copyright-year
+++ b/scripts/check-copyright-year
@@ -2,7 +2,7 @@
#
# Does the copyright statement include the year of the latest git commit?
#
-# Copyright (C) 2012 Codethink Limited
+# Copyright (C) 2012, 2014 Codethink Limited
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -18,6 +18,7 @@
# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+import datetime
import re
import cliapp
@@ -33,23 +34,39 @@ class CheckCopyrightYear(cliapp.Application):
def setup(self):
self.all_ok = True
+ self.uncommitted = self.get_uncommitted_files()
+ self.this_year = datetime.datetime.now().year
def cleanup(self):
if not self.all_ok:
raise cliapp.AppException('Some copyright years need fixing')
+ def get_uncommitted_files(self):
+ filenames = set()
+ status = self.runcmd(['git', 'status', '--porcelain',
'-z'])
+ tokens = status.rstrip('\0').split('\0')
+ while tokens:
+ tok = tokens.pop(0)
+ filenames.add(tok[3:])
+ if 'R' in tok[0:2]:
+ filenames.add(tokens.pop(0))
+ return filenames
+
def process_input_line(self, filename, line):
m = self.pat.match(line)
if not m:
return
- year = self.get_git_commit_year(filename)
+ year = None
+ if filename not in self.uncommitted:
+ year = self.get_git_commit_year(filename)
+
if year is None:
# git does not have a commit date for the file, which might
# happen if the file isn't committed yet. This happens during
# development, and it's OK. It's not quite a lumberjack, but
# let's not get into gender stereotypes here.
- return
+ year = self.this_year
ok = False
for start, end in self.get_copyright_years(m):
--
1.8.5.rc2
1:06 p.m.
New subject: [PATCH 2/2] style check: Include newly added files in checks
---
check | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/check b/check
index a5ef412..77d4e82 100755
--- a/check
+++ b/check
@@ -96,12 +96,13 @@ errors=0
if "$run_style" && [ -d .git ];
then
echo "Checking copyright statements"
- if ! (git ls-files -z | xargs -0r scripts/check-copyright-year); then
+ if ! (git ls-files --cached -z |
+ xargs -0r scripts/check-copyright-year); then
errors=1
fi
echo 'Checking source code for silliness'
- if ! (git ls-files |
+ if ! (git ls-files --cached |
grep -v '\.gz$' |
grep -Ev 'tests[^/]*/.*\.std(out|err)' |
grep -vF 'tests.build/build-system-autotools.script' |
--
1.8.5.rc2
6:21 a.m.
New subject: [PATCH 2/2] style check: Include newly added files in checks
On 19/03/14 18:06, Richard Maw wrote:
---
check | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/check b/check
index a5ef412..77d4e82 100755
--- a/check
+++ b/check
@@ -96,12 +96,13 @@ errors=0
if "$run_style" && [ -d .git ];
then
echo "Checking copyright statements"
- if ! (git ls-files -z | xargs -0r scripts/check-copyright-year); then
+ if ! (git ls-files --cached -z |
+ xargs -0r scripts/check-copyright-year); then
errors=1
fi
echo 'Checking source code for silliness'
- if ! (git ls-files |
+ if ! (git ls-files --cached |
grep -v '\.gz$' |
grep -Ev 'tests[^/]*/.*\.std(out|err)' |
grep -vF 'tests.build/build-system-autotools.script' |
I hate making noise on reviews of simple patches like this. But I'm
confused by the addition of '--cached': 'man git ls-files' says that
this is the default anyway. If I `git add` a file with bad copyright
years and don't commit it, I don't seem to get an error.
--
Sam Thursfield, Codethink Ltd.
Office telephone: +44 161 236 5575
11:37 a.m.
New subject: [PATCH 2/2] style check: Include newly added files in checks
On Tue, Mar 25, 2014 at 11:21:33AM +0000, Sam Thursfield wrote:
I hate making noise on reviews of simple patches like this. But
I'm
confused by the addition of '--cached': 'man git ls-files' says that
this is the default anyway.
Ah, I didn't notice that. I can drop that part of the patch then.
If I `git add` a file with bad copyright years and don't commit
it,
I don't seem to get an error.
That's peculiar, since I just tried it myself and it gave me the
expected error.
12:38 p.m.
On Wed, Mar 19, 2014 at 06:06:13PM +0000, Richard Maw wrote:
Repo: git://git.baserock.org/baserock/baserock/morph.git
Ref: baserock/richardmaw/style-checker
SHA1: 72a420f09835a31eb1ec097f2791a7603120a2d9
Land: origin/master
Haven't been able to test this (laptop busy running Lorry Controller
tests), but code looks OK to me. +1.
--
http://www.codethink.co.uk/ http://wiki.baserock.org/ http://www.baserock.com/
7:44 a.m.
On Wed, Mar 19, 2014 at 06:06:13PM +0000, Richard Maw wrote:
Repo: git://git.baserock.org/baserock/baserock/morph.git
Ref: baserock/richardmaw/style-checker
SHA1: 72a420f09835a31eb1ec097f2791a7603120a2d9
Land: origin/master
This still hasn't been merged, having only gotten a +1, and Sam had some
issues with it.
8:57 a.m.
On Thu, Apr 17, 2014 at 13:44:16 +0100, Richard Maw wrote:
On Wed, Mar 19, 2014 at 06:06:13PM +0000, Richard Maw wrote:
> Repo: git://git.baserock.org/baserock/baserock/morph.git
> Ref: baserock/richardmaw/style-checker
> SHA1: 72a420f09835a31eb1ec097f2791a7603120a2d9
> Land: origin/master
This still hasn't been merged, having only gotten a +1, and Sam had some
issues with it.
I think the patch looks okay, but given the confusion between Sam and yourself
regarding --cached can we check if it was a git version difference?
D.
--
Daniel Silverstone http://www.codethink.co.uk/
Lead Architect GPG 4096/R Key Id: 3CCE BABE 206C 3B69
9:11 a.m.
On 17/04/14 16:57, Daniel Silverstone wrote:
On Thu, Apr 17, 2014 at 13:44:16 +0100, Richard Maw wrote:
> On Wed, Mar 19, 2014 at 06:06:13PM +0000, Richard Maw wrote:
>> Repo: git://git.baserock.org/baserock/baserock/morph.git
>> Ref: baserock/richardmaw/style-checker
>> SHA1: 72a420f09835a31eb1ec097f2791a7603120a2d9
>> Land: origin/master
>
> This still hasn't been merged, having only gotten a +1, and Sam had some
> issues with it.
I think the patch looks okay, but given the confusion between Sam and yourself
regarding --cached can we check if it was a git version difference?
I've just tried to reproduce the problem I described, to `git add` a new
file with bad copyright years and it doesn't raise an error. It raised
an error. So ... sorry for the noise.
I suspect the cause is that the script will only raise an error for
files with a line that matches the copyright year regexp and has
incorrect dates. I probably added a file with no dates, or some such.
(Actually, remove the date field altogether and you get a backtrace with
this script :)
I'm happy for this to be merged, either way. I did not give a -1 and I
do not give one now :)
--
Sam Thursfield, Codethink Ltd.
Office telephone: +44 161 236 5575
11:11 a.m.
On Wed, Mar 19, 2014 at 06:06:13PM +0000, Richard Maw wrote:
Repo: git://git.baserock.org/baserock/baserock/morph.git
Ref: baserock/richardmaw/style-checker
SHA1: 72a420f09835a31eb1ec097f2791a7603120a2d9
Land: origin/master
Changes to morph.git and definitions.git pushed.
3263
days inactive
3292
days old
9 comments
4 participants
participants (4)
-
Daniel Silverstone -
Lars Wirzenius -
Richard Maw -
Sam Thursfield