Code review comment for lp:~stefan.goetz-deactivatedaccount/hipl/commitguard

 review needs-fixing

On Thu, Jan 19, 2012 at 07:36:24AM +0000, Stefan Götz wrote:
> Stefan Götz has proposed merging lp:~stefan.goetz/hipl/commitguard into lp:hipl.

Looks mostly good to me. I don't speak Python, so this review is
necessarily superficial.

> --- .commitguards/author-attribution.guard 1970-01-01 00:00:00 +0000
> +++ .commitguards/author-attribution.guard 2012-01-19 07:35:28 +0000
> @@ -0,0 +1,4 @@
> +[guard]
> +files=\.(c|h|am|ac|in|py|sh)$
> +actions=amr
> +command=if egrep -q "\*.*@author\b" "{}"; then echo "{}: files should not contain author attributions"; false; fi

Do we really need to restrict this to certain file types?

> --- .commitguards/copyright-guard-space.py 1970-01-01 00:00:00 +0000
> +++ .commitguards/copyright-guard-space.py 2012-01-19 07:35:28 +0000
> @@ -0,0 +1,68 @@
> +#!/usr/bin/env python
> +# -*- coding: latin-1 -*-

Please use UTF-8, this is the 21st century (for some time already).
I'm also not sure if you need to tell emacs about this, the file
should be ASCII anyway.

> --- .commitguards/copyright-year.guard 1970-01-01 00:00:00 +0000
> +++ .commitguards/copyright-year.guard 2012-01-19 07:35:28 +0000
> @@ -0,0 +1,4 @@
> +[guard]
> +files=\.(c|h|am|ac|in|py|sh)$
> +actions=amr
> +command=CR_TOKEN=" Copyright \(c\) "; CR_HOLDER=" Aalto "; Y=$(date +%Y); if ! egrep "$CR_TOKEN.*$CR_HOLDER" "{}" | egrep -q "\b$Y\b"; then echo "{}: The copyright statement is not up-to-date"; sed -r "/ Copyright \(c\) / { /\b$Y\b/ ! s/ ([0-9, -]+) ([a-zA-Z]+)/ $Y \2/ }" "{}" | diff -u "{}" - >&2; false; fi
>
> === added file 'tools/bazaar/plugins/commitguard.py'
> --- tools/bazaar/plugins/commitguard.py 1970-01-01 00:00:00 +0000
> +++ tools/bazaar/plugins/commitguard.py 2012-01-19 07:35:28 +0000
> @@ -0,0 +1,353 @@
> +#!/usr/bin/env python
> +# -*- coding: latin-1 -*-

ditto

> +_commitguard_tutorial = """
> +Bazaar plugin for checking code modifications at pre-commit time.
> +
> +About
> +-----
> +
> +Installation
> +------------
> +Copy this file into the bazaar plugin directory (usually .bazaar/plugins/ in

Bazaar

> + The commitguard plugin offers the user to apply such a patch automatically
> + for convenience.

For added convenience the commitguard plugin offers the user the option
of applying such a patch automatically.

Diego