[U-Boot] [PATCH] patman: Allow use outside of u-boot tree

2013-01-09 Thread Vadim Bendebury
To make it usable in git trees not providing a patch checker
implementation, add a command line option, allowing to suppress patch
check. While we are at it, sort debug options alphabetically.

   . unit test passes:
$ ./patman  -t
unittest.result.TestResult run=7 errors=0 failures=0
   . successfully used patman in the autotest tree to generate a patch
 email (with --no-check option)
   . successfully used patman in the u-boot tree to generate a patch
 email
   . `patman --help' now shows command line options ordered
 alphabetically

Signed-off-by: Vadim Bendebury vben...@chromium.org
---
 tools/patman/patman.py |   14 ++
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index e56dd01..6620a48 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -50,6 +50,9 @@ parser.add_option('-i', '--ignore-errors', 
action='store_true',
help='Send patches email even if patch errors are found')
 parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run',
default=False, help=Do a try run (create but don't email patches))
+parser.add_option('-p', '--project', default=project.DetectProject(),
+  help=Project name; affects default option values and 
+  aliases [default: %default])
 parser.add_option('-s', '--start', dest='start', type='int',
default=0, help='Commit to start creating patches from (0 = HEAD)')
 parser.add_option('-t', '--test', action='store_true', dest='test',
@@ -58,11 +61,11 @@ parser.add_option('-v', '--verbose', action='store_true', 
dest='verbose',
default=False, help='Verbose output of errors and warnings')
 parser.add_option('--cc-cmd', dest='cc_cmd', type='string', action='store',
default=None, help='Output cc list for patch file (used by git)')
+parser.add_option('--no-check', action='store_true', dest='no_check',
+  default=False,
+  help=Don't check for patch compliance)
 parser.add_option('--no-tags', action='store_false', dest='process_tags',
   default=True, help=Don't process subject tags as aliaes)
-parser.add_option('-p', '--project', default=project.DetectProject(),
-  help=Project name; affects default option values and 
-  aliases [default: %default])
 
 parser.usage = patman [options]
 
@@ -146,7 +149,10 @@ else:
 series.DoChecks()
 
 # Check the patches, and run them through 'git am' just to be sure
-ok = checkpatch.CheckPatches(options.verbose, args)
+if options.no_check:
+ok = True
+else:
+ok = checkpatch.CheckPatches(options.verbose, args)
 if not gitutil.ApplyPatches(options.verbose, args,
 options.count + options.start):
 ok = False
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] patman: Allow use outside of u-boot tree

2013-01-09 Thread Vadim Bendebury
On Wed, Jan 9, 2013 at 2:01 PM, Vadim Bendebury vben...@chromium.org wrote:
 To make it usable in git trees not providing a patch checker
 implementation, add a command line option, allowing to suppress patch
 check. While we are at it, sort debug options alphabetically.

. unit test passes:
 $ ./patman  -t
 unittest.result.TestResult run=7 errors=0 failures=0
. successfully used patman in the autotest tree to generate a patch
  email (with --no-check option)
. successfully used patman in the u-boot tree to generate a patch
  email
. `patman --help' now shows command line options ordered
  alphabetically

 Signed-off-by: Vadim Bendebury vben...@chromium.org
 ---
  tools/patman/patman.py |   14 ++
  1 files changed, 10 insertions(+), 4 deletions(-)

 diff --git a/tools/patman/patman.py b/tools/patman/patman.py
 index e56dd01..6620a48 100755
 --- a/tools/patman/patman.py
 +++ b/tools/patman/patman.py
 @@ -50,6 +50,9 @@ parser.add_option('-i', '--ignore-errors', 
 action='store_true',
 help='Send patches email even if patch errors are found')
  parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run',
 default=False, help=Do a try run (create but don't email patches))
 +parser.add_option('-p', '--project', default=project.DetectProject(),
 +  help=Project name; affects default option values and 
 +  aliases [default: %default])
  parser.add_option('-s', '--start', dest='start', type='int',
 default=0, help='Commit to start creating patches from (0 = HEAD)')
  parser.add_option('-t', '--test', action='store_true', dest='test',
 @@ -58,11 +61,11 @@ parser.add_option('-v', '--verbose', action='store_true', 
 dest='verbose',
 default=False, help='Verbose output of errors and warnings')
  parser.add_option('--cc-cmd', dest='cc_cmd', type='string', action='store',
 default=None, help='Output cc list for patch file (used by git)')
 +parser.add_option('--no-check', action='store_true', dest='no_check',
 +  default=False,
 +  help=Don't check for patch compliance)
  parser.add_option('--no-tags', action='store_false', dest='process_tags',
default=True, help=Don't process subject tags as aliaes)
 -parser.add_option('-p', '--project', default=project.DetectProject(),
 -  help=Project name; affects default option values and 
 -  aliases [default: %default])

  parser.usage = patman [options]

 @@ -146,7 +149,10 @@ else:
  series.DoChecks()

  # Check the patches, and run them through 'git am' just to be sure
 -ok = checkpatch.CheckPatches(options.verbose, args)
 +if options.no_check:
 +ok = True
 +else:
 +ok = checkpatch.CheckPatches(options.verbose, args)
  if not gitutil.ApplyPatches(options.verbose, args,
  options.count + options.start):
  ok = False
 --
 1.7.7.3


Doug, thank you for a prompt review, copying your response here,
please see below:

On Wed, Jan 9, 2013 at 1:48 PM, Doug Anderson diand...@chromium.org wrote:
 Vadim,

 Thanks for the patch!  Looks good in general, though please add the
 patman prefix to the first line of your commit message.


done


 On Wed, Jan 9, 2013 at 1:13 PM, Vadim Bendebury vben...@chromium.org wrote:
 To make it usable in git trees not providing a patch checker
 implementation, add a command line option, allowing to suippress patch

 s/suippress/suppress


done

 +parser.add_option('--no-check', action='store_true', dest='no_check',
 +  default=False,
 +  help=Don't check for patch compliance)

 IMHO It would be slightly better to use action='store_false',
 dest='check', and default=True (just to avoid so many
 double-negatives).

I don't quite agree with this part - I think it's perfectly reasonable
to use 'no-check' to suppress the check, just as well as to use
'no-tags' to suppress interpreting tags.

`--no' communicates that by default the respective feature is enabled,
and to disable it one needs to add a command line option with no
parameter.

cheers,
/vb
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] patman: Allow use outside of u-boot tree

2013-01-09 Thread Vadim Bendebury
On Wed, Jan 9, 2013 at 2:57 PM, Doug Anderson diand...@chromium.org wrote:
 Vadim,

 On Wed, Jan 9, 2013 at 2:07 PM, Vadim Bendebury vben...@chromium.org wrote:
 +parser.add_option('--no-check', action='store_true', dest='no_check',
 +  default=False,
 +  help=Don't check for patch compliance)

 IMHO It would be slightly better to use action='store_false',
 dest='check', and default=True (just to avoid so many
 double-negatives).

 I don't quite agree with this part - I think it's perfectly reasonable
 to use 'no-check' to suppress the check, just as well as to use
 'no-tags' to suppress interpreting tags.

 `--no' communicates that by default the respective feature is enabled,
 and to disable it one needs to add a command line option with no
 parameter.

 Sorry--should have been more explicit.  Was still expecting the option
 to be --no-check.  Just asking for a change to the way it's stored.
 Like this in the python dev guide:

 parser.add_option(--clobber, action=store_true, dest=clobber)
 parser.add_option(--no-clobber, action=store_false, dest=clobber)

 In your case, I don't think you need to add the check option too,
 but just store to the check option:

 parser.add_option('--no-check', action='store_false', dest='check',
   default=True,
   help=Don't check for patch compliance)

ah, a good idea, changed. Also, modified the code in checkpatch.py not
to throw an exception, but to print an error message and return a
nonzero status.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] patman: Allow use outside of u-boot tree

2013-01-09 Thread Doug Anderson
Vadim,

On Wed, Jan 9, 2013 at 2:07 PM, Vadim Bendebury vben...@chromium.org wrote:
 +parser.add_option('--no-check', action='store_true', dest='no_check',
 +  default=False,
 +  help=Don't check for patch compliance)

 IMHO It would be slightly better to use action='store_false',
 dest='check', and default=True (just to avoid so many
 double-negatives).

 I don't quite agree with this part - I think it's perfectly reasonable
 to use 'no-check' to suppress the check, just as well as to use
 'no-tags' to suppress interpreting tags.

 `--no' communicates that by default the respective feature is enabled,
 and to disable it one needs to add a command line option with no
 parameter.

Sorry--should have been more explicit.  Was still expecting the option
to be --no-check.  Just asking for a change to the way it's stored.
Like this in the python dev guide:

parser.add_option(--clobber, action=store_true, dest=clobber)
parser.add_option(--no-clobber, action=store_false, dest=clobber)

In your case, I don't think you need to add the check option too,
but just store to the check option:

parser.add_option('--no-check', action='store_false', dest='check',
  default=True,
  help=Don't check for patch compliance)
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] patman: Allow use outside of u-boot tree

2013-01-09 Thread Vadim Bendebury
To make it usable in git trees not providing a patch checker
implementation, add a command line option, allowing to suppress patch
check. While we are at it, sort debug options alphabetically.

Also, do not raise an exception if checkpatch.pl is not found - just
print an error message suggesting to use the new option, and return
nonzero status.

   . unit test passes:
$ ./patman  -t
unittest.result.TestResult run=7 errors=0 failures=0
   . successfully used patman in the autotest tree to generate a patch
 email (with --no-check option)
   . successfully used patman in the u-boot tree to generate a patch
 email
   . `patman --help' now shows command line options ordered
 alphabetically

Signed-off-by: Vadim Bendebury vben...@chromium.org
---
 tools/patman/checkpatch.py |   10 +-
 tools/patman/patman.py |   14 ++
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
index f72f8ee..00aef09 100644
--- a/tools/patman/checkpatch.py
+++ b/tools/patman/checkpatch.py
@@ -23,6 +23,7 @@ import command
 import gitutil
 import os
 import re
+import sys
 import terminal
 
 def FindCheckPatch():
@@ -47,8 +48,10 @@ def FindCheckPatch():
 if os.path.isfile(fname):
 return fname
 path = os.path.dirname(path)
-print 'Could not find checkpatch.pl'
-return None
+
+print  sys.stderr, ('Cannot find checkpatch.pl - please put it in your ' 
+
+'~/bin directory or use --no_patch')
+sys.exit(1)
 
 def CheckPatch(fname, verbose=False):
 Run checkpatch.pl on a file.
@@ -67,9 +70,6 @@ def CheckPatch(fname, verbose=False):
 error_count, warning_count, lines = 0, 0, 0
 problems = []
 chk = FindCheckPatch()
-if not chk:
-raise OSError, ('Cannot find checkpatch.pl - please put it in your ' +
-'~/bin directory')
 item = {}
 stdout = command.Output(chk, '--no-tree', fname)
 #pipe = subprocess.Popen(cmd, stdout=subprocess.PIPE)
diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index e56dd01..e049081 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -50,6 +50,9 @@ parser.add_option('-i', '--ignore-errors', 
action='store_true',
help='Send patches email even if patch errors are found')
 parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run',
default=False, help=Do a try run (create but don't email patches))
+parser.add_option('-p', '--project', default=project.DetectProject(),
+  help=Project name; affects default option values and 
+  aliases [default: %default])
 parser.add_option('-s', '--start', dest='start', type='int',
default=0, help='Commit to start creating patches from (0 = HEAD)')
 parser.add_option('-t', '--test', action='store_true', dest='test',
@@ -58,11 +61,11 @@ parser.add_option('-v', '--verbose', action='store_true', 
dest='verbose',
default=False, help='Verbose output of errors and warnings')
 parser.add_option('--cc-cmd', dest='cc_cmd', type='string', action='store',
default=None, help='Output cc list for patch file (used by git)')
+parser.add_option('--no-check', action='store_false', dest='check_patch',
+  default=True,
+  help=Don't check for patch compliance)
 parser.add_option('--no-tags', action='store_false', dest='process_tags',
   default=True, help=Don't process subject tags as aliaes)
-parser.add_option('-p', '--project', default=project.DetectProject(),
-  help=Project name; affects default option values and 
-  aliases [default: %default])
 
 parser.usage = patman [options]
 
@@ -146,7 +149,10 @@ else:
 series.DoChecks()
 
 # Check the patches, and run them through 'git am' just to be sure
-ok = checkpatch.CheckPatches(options.verbose, args)
+if options.check_patch:
+ok = checkpatch.CheckPatches(options.verbose, args)
+else:
+ok = True
 if not gitutil.ApplyPatches(options.verbose, args,
 options.count + options.start):
 ok = False
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot