| Message ID | 1357766013-25729-1-git-send-email-vbendeb@chromium.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wed, Jan 9, 2013 at 2:13 PM, Simon Glass <sjg@google.com> wrote: > Hi Vadim, > > Looks good! Please can you add a single character option? > Simon, I could not think of a good single letter option to pick, so I did not, but if you have a suggestion I implement it. > Can you also please add an option to skip the 'apply' step? This takes > quite a bit of time, and it would be nice to have a 'fast' option. > does it have to be in this CL? cheers, /vb > Regards, > Simon > > On Wed, Jan 9, 2013 at 1:48 PM, Doug Anderson <dianders@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. >> >> >> On Wed, Jan 9, 2013 at 1:13 PM, Vadim Bendebury <vbendeb@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 >> >>> +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). >> >> >> -Doug
Hi Vadim, On Wed, Jan 9, 2013 at 2:16 PM, Vadim Bendebury <vbendeb@chromium.org> wrote: > On Wed, Jan 9, 2013 at 2:13 PM, Simon Glass <sjg@google.com> wrote: >> Hi Vadim, >> >> Looks good! Please can you add a single character option? >> > > Simon, I could not think of a good single letter option to pick, so I > did not, but if you have a suggestion I implement it. I can't think of a good one. -C or -P ? > >> Can you also please add an option to skip the 'apply' step? This takes >> quite a bit of time, and it would be nice to have a 'fast' option. >> > > does it have to be in this CL? > No not at all. > cheers, > /vb Regards, Simon > >> Regards, >> Simon >> >> On Wed, Jan 9, 2013 at 1:48 PM, Doug Anderson <dianders@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. >>> >>> >>> On Wed, Jan 9, 2013 at 1:13 PM, Vadim Bendebury <vbendeb@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 >>> >>>> +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). >>> >>> >>> -Doug
On Wed, Jan 9, 2013 at 2:22 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Vadim, > > On Wed, Jan 9, 2013 at 2:16 PM, Vadim Bendebury <vbendeb@chromium.org> wrote: >> On Wed, Jan 9, 2013 at 2:13 PM, Simon Glass <sjg@google.com> wrote: >>> Hi Vadim, >>> >>> Looks good! Please can you add a single character option? >>> >> >> Simon, I could not think of a good single letter option to pick, so I >> did not, but if you have a suggestion I implement it. > > I can't think of a good one. > > -C or -P ? > Yeah, I feel a bit awkward about these, also, it would be inconsistent with --no-tags which does not have a single letter alternative. Let's see what others think about it - I would rather leave it as is... cheers, /v >> >>> Can you also please add an option to skip the 'apply' step? This takes >>> quite a bit of time, and it would be nice to have a 'fast' option. >>> >> >> does it have to be in this CL? >> > > No not at all. > >> cheers, >> /vb > > Regards, > Simon > >> >>> Regards, >>> Simon >>> >>> On Wed, Jan 9, 2013 at 1:48 PM, Doug Anderson <dianders@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. >>>> >>>> >>>> On Wed, Jan 9, 2013 at 1:13 PM, Vadim Bendebury <vbendeb@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 >>>> >>>>> +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). >>>> >>>> >>>> -Doug
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
To make it usable in git trees not providing a patch checker implementation, add a command line option, allowing to suippress 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 <vbendeb@chromium.org> --- tools/patman/patman.py | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-)