You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Rice, Kevin (Contractor)" <Ke...@searshc.com> on 2013/05/10 17:48:38 UTC

[PATCH] Add contrib/hook-scripts pre-commit for python requiring tabs and svn properties

[[[
Add contrib/hook-scripts/precommit-pyfiles-notabs-properties.py

Functions as a pre-commit hook script that does several things:
    * prevents committing any python file containing a tab character
    * checks if there are tabs in the source file and warns if so
    * aborts if incorrect properties of eol-style and keywords 'id'
]]]


This message, including any attachments, is the property of Sears Holdings Corporation and/or one of its subsidiaries. It is confidential and may contain proprietary or legally privileged information. If you are not the intended recipient, please delete it without reading the contents. Thank you.

Re: [PATCH] Add contrib/hook-scripts pre-commit for python requiring tabs and svn properties

Posted by Ben Reser <be...@reser.org>.
On Fri, May 10, 2013 at 11:11 AM, Daniel Shahaf <da...@elego.de> wrote:
> Rice, Kevin (Contractor) wrote on Fri, May 10, 2013 at 15:48:38 +0000:
>> This message, including any attachments, is the property of Sears
> ...
>> without reading the contents. Thank you.
>
> Please nuke these footers when emailing public mailing lists.

Sadly he may not be able to.  But more importantly any contribution
that comes with that footer attached becomes hard to accept because it
contradicts any license you may attach to the code.  As such I'd say
that you'd probably need to get a Contributor License Agreement on
file in order to contribute this code:
http://www.apache.org/licenses/

However, I agree with Daniel's other point that I don't think this is
sufficiently generic enough to warrant committing to our repo.  We do
now have a script intended to make it easy to apply specific rules to
certain files:
http://svn.apache.org/repos/asf/subversion/trunk/tools/hook-scripts/validate-files.py

So in this case I'd probably configure that tool to use some subset of
what is in your script to do the specific tests you want (tab,
eol-style, etc).

Re: [PATCH] Add contrib/hook-scripts pre-commit for python requiring tabs and svn properties

Posted by Daniel Shahaf <da...@elego.de>.
Rice, Kevin (Contractor) wrote on Fri, May 10, 2013 at 15:48:38 +0000:
> [[[
> Add contrib/hook-scripts/precommit-pyfiles-notabs-properties.py
> 
> Functions as a pre-commit hook script that does several things:
>     * prevents committing any python file containing a tab character
>     * checks if there are tabs in the source file and warns if so
>     * aborts if incorrect properties of eol-style and keywords 'id'
> ]]]
> 

Thanks for the patch.  I'm not sure if this is sufficiently generic for
inclusion in contrib/ (especially given that we deprecate that
directory), but I'll note a few points anyway:

> 
> This message, including any attachments, is the property of Sears
...
> without reading the contents. Thank you.

Please nuke these footers when emailing public mailing lists.

Content-Description: contrib.precommit.pyfiles.patch.txt
> Index: contrib/hook-scripts/precommit-pyfiles-notabs-properties.py
> ===================================================================
> --- contrib/hook-scripts/precommit-pyfiles-notabs-properties.py	(revision 0)
> +++ contrib/hook-scripts/precommit-pyfiles-notabs-properties.py	(working copy)
> @@ -0,0 +1,127 @@
> +#!/bin/env python
> +

Please specify a license.

Also, the conventional #! line uses '/usr/bin/env', not '/bin/env'.  In
fact, the latter does not exist on either FreeBSD or Linux.

> +def command_output(cmd):
> +    " Capture a command's standard output. "
> +    import subprocess
> +    return subprocess.Popen(
> +        cmd.split(), stdout=subprocess.PIPE).communicate()[0]

This will not DTRT if you have filenames that contain spaces or
children of the root with names that start with a hyphen.  That should
be fixed (preferred) or documented.