* Re: [gentoo-portage-dev] [PATCH] Add --output-style option to repoman
2014-02-10 22:57 [gentoo-portage-dev] [PATCH] Add --output-style option to repoman Chris Reffett
@ 2014-02-11 0:16 ` Brian Dolbec
2014-02-11 0:35 ` Alec Warner
2014-02-11 1:22 ` [gentoo-portage-dev] [PATCH v2] " Chris Reffett
2 siblings, 0 replies; 12+ messages in thread
From: Brian Dolbec @ 2014-02-11 0:16 UTC (permalink / raw
To: gentoo-portage-dev
On Mon, 10 Feb 2014 17:57:26 -0500
Chris Reffett <creffett@gentoo.org> wrote:
> This patch adds a --output-style option to repoman, which gives the
> user a choice of output formats for the repoman checks. Choices are
> "default" (current style) and "column" (a greppable format), but it
> should be easy to add more. Fixes bug 481584.
> ---
> bin/repoman | 18 +++++++++++++++++-
> pym/repoman/utilities.py | 38 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/bin/repoman b/bin/repoman
> index 3504b6b..957ee08 100755
> --- a/bin/repoman
> +++ b/bin/repoman
> @@ -144,9 +144,17 @@ def ParseArgs(argv, qahelp):
> 'scan' : 'Scan directory tree for QA issues'
> }
>
> + output_choices = {
> + 'default' : 'The normal output format',
> + 'column' : 'Columnar output suitable for use with
> grep'
> + }
> +
> mode_keys = list(modes)
> mode_keys.sort()
>
> + output_keys = list(output_choices)
> + output_keys.sort
> +
> parser = ArgumentParser(usage="repoman [options] [mode]",
> description="Modes: %s" % " | ".join(mode_keys),
> epilog="For more help consult the man page.")
> @@ -228,6 +236,9 @@ def ParseArgs(argv, qahelp):
> parser.add_argument('--without-mask', dest='without_mask',
> action='store_true', default=False, help='behave as if no
> package.mask entries exist (not allowed with commit mode)')
> + parser.add_argument('--output-style', dest='output_style',
> choices=output_keys,
> + help='select output type')
Why not set the default right here?
+ parser.add_argument('--output-style', dest='output_style',
choices=output_keys, default='default',
+ help='select output type')
> +
> parser.add_argument('--mode', dest='mode', choices=mode_keys,
> help='specify which mode repoman will run in
> (default=full)')
> @@ -256,6 +267,8 @@ def ParseArgs(argv, qahelp):
> if opts.mode == 'ci':
> opts.mode = 'commit' # backwards compat shortcut
>
> + if not opts.output_style:
> + opts.output_style = 'default' #default to the
standard output type
delete ^^^ not needed if you set the default as above
># Use the verbosity and quiet options to fiddle
> with the loglevel appropriately for val in range(opts.verbosity):
> logger = logging.getLogger()
> @@ -2422,7 +2435,10 @@ console_writer.style_listener =
> style_file.new_styles
> f = formatter.AbstractFormatter(console_writer)
>
> -utilities.format_qa_output(f, stats, fails, dofull, dofail, options,
> qawarnings) +if options.output_style == 'column':
> + utilities.format_qa_output_column(f, stats, fails, dofull,
> dofail, options, qawarnings) +else:
> + utilities.format_qa_output(f, stats, fails, dofull, dofail,
> options, qawarnings)
> style_file.flush()
> del console_writer, f, style_file
> diff --git a/pym/repoman/utilities.py b/pym/repoman/utilities.py
> index 3ec3a4a..713f208 100644
> --- a/pym/repoman/utilities.py
> +++ b/pym/repoman/utilities.py
> @@ -330,6 +330,44 @@ def format_qa_output(formatter, stats, fails,
> dofull, dofail, options, qawarning formatter.add_line_break()
>
>
> +def format_qa_output_column(formatter, stats, fails, dofull, dofail,
> options, qawarnings):
> + """Helper function that formats output in a
> machine-parseable column format +
> + Args:
> + formatter - a subclass of Formatter
> + stats - a dict of qa status items
> + fails - a dict of qa status failures
> + dofull - boolean to print full results or a summary
> + dofail - boolean to decide if failure was hard or
> soft +
> + Returns:
> + None (modifies formatter)
> + """
I thought we had a doc around that stated the format for docstrings.
I recall it being a different format that the above parameter descriptions.
> + full = options.mode == 'full'
> + for category, number in stats.items():
> + # we only want key value pairs where value > 0
> + if number < 1:
> + continue
> +
> + formatter.add_literal_data("NumberOf " + category +
> " ")
> + if category in qawarnings:
> + formatter.push_style("WARN")
> + else:
> + formatter.push_style("BAD")
> + formatter.add_literal_data("%s" % number)
> + formatter.pop_style()
> + formatter.add_line_break()
> + if not dofull:
> + if not full and dofail and category in
> qawarnings:
> + # warnings are considered noise when
> there are failures
> + continue
> + fails_list = fails[category]
> + if not full and len(fails_list) > 12:
> + fails_list = fails_list[:12]
> + for failure in fails_list:
> + formatter.add_literal_data(category
> + " " + failure)
> + formatter.add_line_break()
> +
> def editor_is_executable(editor):
> """
> Given an EDITOR string, validate that it refers to
--
Brian Dolbec <dolsen>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [gentoo-portage-dev] [PATCH] Add --output-style option to repoman
2014-02-10 22:57 [gentoo-portage-dev] [PATCH] Add --output-style option to repoman Chris Reffett
2014-02-11 0:16 ` Brian Dolbec
@ 2014-02-11 0:35 ` Alec Warner
2014-02-11 1:22 ` [gentoo-portage-dev] [PATCH v2] " Chris Reffett
2 siblings, 0 replies; 12+ messages in thread
From: Alec Warner @ 2014-02-11 0:35 UTC (permalink / raw
To: gentoo-portage-dev
[-- Attachment #1: Type: text/plain, Size: 5217 bytes --]
On Mon, Feb 10, 2014 at 2:57 PM, Chris Reffett <creffett@gentoo.org> wrote:
> This patch adds a --output-style option to repoman, which gives the user
> a choice of output formats for the repoman checks. Choices are "default"
> (current style) and "column" (a greppable format), but it should be easy
> to add more. Fixes bug 481584.
> ---
> bin/repoman | 18 +++++++++++++++++-
> pym/repoman/utilities.py | 38 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/bin/repoman b/bin/repoman
> index 3504b6b..957ee08 100755
> --- a/bin/repoman
> +++ b/bin/repoman
> @@ -144,9 +144,17 @@ def ParseArgs(argv, qahelp):
> 'scan' : 'Scan directory tree for QA issues'
> }
>
> + output_choices = {
> + 'default' : 'The normal output format',
> + 'column' : 'Columnar output suitable for use with grep'
> + }
> +
> mode_keys = list(modes)
> mode_keys.sort()
>
>
output_keys = sorted(output_choices)
?
> + output_keys = list(output_choices)
> + output_keys.sort
> +
> parser = ArgumentParser(usage="repoman [options] [mode]",
> description="Modes: %s" % " | ".join(mode_keys),
> epilog="For more help consult the man page.")
> @@ -228,6 +236,9 @@ def ParseArgs(argv, qahelp):
> parser.add_argument('--without-mask', dest='without_mask',
> action='store_true',
> default=False, help='behave as if no package.mask entries
> exist (not allowed with commit mode)')
>
> + parser.add_argument('--output-style', dest='output_style',
> choices=output_keys,
> + help='select output type')
> +
> parser.add_argument('--mode', dest='mode', choices=mode_keys,
> help='specify which mode repoman will run in
> (default=full)')
>
> @@ -256,6 +267,8 @@ def ParseArgs(argv, qahelp):
> if opts.mode == 'ci':
> opts.mode = 'commit' # backwards compat shortcut
>
> + if not opts.output_style:
> + opts.output_style = 'default' #default to the standard
> output type
> # Use the verbosity and quiet options to fiddle with the loglevel
> appropriately
> for val in range(opts.verbosity):
> logger = logging.getLogger()
> @@ -2422,7 +2435,10 @@ console_writer.style_listener =
> style_file.new_styles
>
> f = formatter.AbstractFormatter(console_writer)
>
> -utilities.format_qa_output(f, stats, fails, dofull, dofail, options,
> qawarnings)
> +if options.output_style == 'column':
> + utilities.format_qa_output_column(f, stats, fails, dofull, dofail,
> options, qawarnings)
> +else:
> + utilities.format_qa_output(f, stats, fails, dofull, dofail,
> options, qawarnings)
>
> style_file.flush()
> del console_writer, f, style_file
> diff --git a/pym/repoman/utilities.py b/pym/repoman/utilities.py
> index 3ec3a4a..713f208 100644
> --- a/pym/repoman/utilities.py
> +++ b/pym/repoman/utilities.py
> @@ -330,6 +330,44 @@ def format_qa_output(formatter, stats, fails, dofull,
> dofail, options, qawarning
> formatter.add_line_break()
>
>
> +def format_qa_output_column(formatter, stats, fails, dofull, dofail,
> options, qawarnings):
> + """Helper function that formats output in a machine-parseable
> column format
> +
> + Args:
> + formatter - a subclass of Formatter
> + stats - a dict of qa status items
> + fails - a dict of qa status failures
> + dofull - boolean to print full results or a summary
> + dofail - boolean to decide if failure was hard or soft
>
missing options and qawarnings ?
> +
> + Returns:
> + None (modifies formatter)
> + """
> + full = options.mode == 'full'
> + for category, number in stats.items():
> + # we only want key value pairs where value > 0
> + if number < 1:
> + continue
> +
> + formatter.add_literal_data("NumberOf " + category + " ")
> + if category in qawarnings:
> + formatter.push_style("WARN")
> + else:
> + formatter.push_style("BAD")
> + formatter.add_literal_data("%s" % number)
> + formatter.pop_style()
> + formatter.add_line_break()
> + if not dofull:
> + if not full and dofail and category in qawarnings:
> + # warnings are considered noise when there
> are failures
> + continue
> + fails_list = fails[category]
> + if not full and len(fails_list) > 12:
> + fails_list = fails_list[:12]
> + for failure in fails_list:
> + formatter.add_literal_data(category + " "
> + failure)
> + formatter.add_line_break()
> +
> def editor_is_executable(editor):
> """
> Given an EDITOR string, validate that it refers to
> --
> 1.8.5.3
>
>
>
[-- Attachment #2: Type: text/html, Size: 7019 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [gentoo-portage-dev] [PATCH v2] Add --output-style option to repoman
2014-02-10 22:57 [gentoo-portage-dev] [PATCH] Add --output-style option to repoman Chris Reffett
2014-02-11 0:16 ` Brian Dolbec
2014-02-11 0:35 ` Alec Warner
@ 2014-02-11 1:22 ` Chris Reffett
2014-02-11 6:35 ` Brian Dolbec
2014-02-13 8:19 ` Mike Frysinger
2 siblings, 2 replies; 12+ messages in thread
From: Chris Reffett @ 2014-02-11 1:22 UTC (permalink / raw
To: gentoo-portage-dev
This patch adds a --output-style option to repoman, which gives the user
a choice of output formats for the repoman checks. Choices are "default"
(current style) and "column" (a greppable format), but it should be easy
to add more. Fixes bug 481584.
v2: Fix docstring to be complete and in the standard format, make use of
default choices in --output-style wrt comments by antarus and dol-sen
---
bin/repoman | 15 ++++++++++++++-
pym/repoman/utilities.py | 44 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/bin/repoman b/bin/repoman
index 3504b6b..c7a1c4c 100755
--- a/bin/repoman
+++ b/bin/repoman
@@ -144,9 +144,16 @@ def ParseArgs(argv, qahelp):
'scan' : 'Scan directory tree for QA issues'
}
+ output_choices = {
+ 'default' : 'The normal output format',
+ 'column' : 'Columnar output suitable for use with grep'
+ }
+
mode_keys = list(modes)
mode_keys.sort()
+ output_keys = sorted(output_choices)
+
parser = ArgumentParser(usage="repoman [options] [mode]",
description="Modes: %s" % " | ".join(mode_keys),
epilog="For more help consult the man page.")
@@ -228,6 +235,9 @@ def ParseArgs(argv, qahelp):
parser.add_argument('--without-mask', dest='without_mask', action='store_true',
default=False, help='behave as if no package.mask entries exist (not allowed with commit mode)')
+ parser.add_argument('--output-style', dest='output_style', choices=output_keys,
+ help='select output type', default='default')
+
parser.add_argument('--mode', dest='mode', choices=mode_keys,
help='specify which mode repoman will run in (default=full)')
@@ -2422,7 +2432,10 @@ console_writer.style_listener = style_file.new_styles
f = formatter.AbstractFormatter(console_writer)
-utilities.format_qa_output(f, stats, fails, dofull, dofail, options, qawarnings)
+if options.output_style == 'column':
+ utilities.format_qa_output_column(f, stats, fails, dofull, dofail, options, qawarnings)
+else:
+ utilities.format_qa_output(f, stats, fails, dofull, dofail, options, qawarnings)
style_file.flush()
del console_writer, f, style_file
diff --git a/pym/repoman/utilities.py b/pym/repoman/utilities.py
index 3ec3a4a..aec61fe 100644
--- a/pym/repoman/utilities.py
+++ b/pym/repoman/utilities.py
@@ -330,6 +330,50 @@ def format_qa_output(formatter, stats, fails, dofull, dofail, options, qawarning
formatter.add_line_break()
+def format_qa_output_column(formatter, stats, fails, dofull, dofail, options, qawarnings):
+ """Helper function that formats output in a machine-parseable column format
+
+ @param formatter: an instance of Formatter
+ @type formatter: Formatter
+ @param path: dict of qa status items
+ @type path: dict
+ @param fails: dict of qa status failures
+ @type fails: dict
+ @param dofull: Whether to print full results or a summary
+ @type dofull: boolean
+ @param dofail: Whether failure was hard or soft
+ @type dofail: boolean
+ @param options: The command-line options provided to repoman
+ @type options: Namespace
+ @param qawarnings: the set of warning types
+ @type qawarnings: set
+ @return: None (modifies formatter)
+ """
+ full = options.mode == 'full'
+ for category, number in stats.items():
+ # we only want key value pairs where value > 0
+ if number < 1:
+ continue
+
+ formatter.add_literal_data("NumberOf " + category + " ")
+ if category in qawarnings:
+ formatter.push_style("WARN")
+ else:
+ formatter.push_style("BAD")
+ formatter.add_literal_data("%s" % number)
+ formatter.pop_style()
+ formatter.add_line_break()
+ if not dofull:
+ if not full and dofail and category in qawarnings:
+ # warnings are considered noise when there are failures
+ continue
+ fails_list = fails[category]
+ if not full and len(fails_list) > 12:
+ fails_list = fails_list[:12]
+ for failure in fails_list:
+ formatter.add_literal_data(category + " " + failure)
+ formatter.add_line_break()
+
def editor_is_executable(editor):
"""
Given an EDITOR string, validate that it refers to
--
1.8.5.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [gentoo-portage-dev] [PATCH v2] Add --output-style option to repoman
2014-02-11 1:22 ` [gentoo-portage-dev] [PATCH v2] " Chris Reffett
@ 2014-02-11 6:35 ` Brian Dolbec
2014-02-13 8:19 ` Mike Frysinger
1 sibling, 0 replies; 12+ messages in thread
From: Brian Dolbec @ 2014-02-11 6:35 UTC (permalink / raw
To: gentoo-portage-dev
On Mon, 10 Feb 2014 20:22:36 -0500
Chris Reffett <creffett@gentoo.org> wrote:
> This patch adds a --output-style option to repoman, which gives the
> user a choice of output formats for the repoman checks. Choices are
> "default" (current style) and "column" (a greppable format), but it
> should be easy to add more. Fixes bug 481584.
>
> v2: Fix docstring to be complete and in the standard format, make use
> of default choices in --output-style wrt comments by antarus and
> dol-sen ---
> bin/repoman | 15 ++++++++++++++-
> pym/repoman/utilities.py | 44
> ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58
> insertions(+), 1 deletion(-)
>
> diff --git a/bin/repoman b/bin/repoman
> index 3504b6b..c7a1c4c 100755
> --- a/bin/repoman
> +++ b/bin/repoman
> @@ -144,9 +144,16 @@ def ParseArgs(argv, qahelp):
> 'scan' : 'Scan directory tree for QA issues'
> }
>
> + output_choices = {
> + 'default' : 'The normal output format',
> + 'column' : 'Columnar output suitable for use with
> grep'
> + }
> +
> mode_keys = list(modes)
> mode_keys.sort()
>
> + output_keys = sorted(output_choices)
> +
> parser = ArgumentParser(usage="repoman [options] [mode]",
> description="Modes: %s" % " | ".join(mode_keys),
> epilog="For more help consult the man page.")
> @@ -228,6 +235,9 @@ def ParseArgs(argv, qahelp):
> parser.add_argument('--without-mask', dest='without_mask',
> action='store_true', default=False, help='behave as if no
> package.mask entries exist (not allowed with commit mode)')
> + parser.add_argument('--output-style', dest='output_style',
> choices=output_keys,
> + help='select output type', default='default')
> +
> parser.add_argument('--mode', dest='mode', choices=mode_keys,
> help='specify which mode repoman will run in
> (default=full)')
> @@ -2422,7 +2432,10 @@ console_writer.style_listener =
> style_file.new_styles
> f = formatter.AbstractFormatter(console_writer)
>
> -utilities.format_qa_output(f, stats, fails, dofull, dofail, options,
> qawarnings) +if options.output_style == 'column':
> + utilities.format_qa_output_column(f, stats, fails, dofull,
> dofail, options, qawarnings) +else:
> + utilities.format_qa_output(f, stats, fails, dofull, dofail,
> options, qawarnings)
> style_file.flush()
> del console_writer, f, style_file
> diff --git a/pym/repoman/utilities.py b/pym/repoman/utilities.py
> index 3ec3a4a..aec61fe 100644
> --- a/pym/repoman/utilities.py
> +++ b/pym/repoman/utilities.py
> @@ -330,6 +330,50 @@ def format_qa_output(formatter, stats, fails,
> dofull, dofail, options, qawarning formatter.add_line_break()
>
>
> +def format_qa_output_column(formatter, stats, fails, dofull, dofail,
> options, qawarnings):
> + """Helper function that formats output in a
> machine-parseable column format +
> + @param formatter: an instance of Formatter
> + @type formatter: Formatter
> + @param path: dict of qa status items
> + @type path: dict
> + @param fails: dict of qa status failures
> + @type fails: dict
> + @param dofull: Whether to print full results or a summary
> + @type dofull: boolean
> + @param dofail: Whether failure was hard or soft
> + @type dofail: boolean
> + @param options: The command-line options provided to repoman
> + @type options: Namespace
> + @param qawarnings: the set of warning types
> + @type qawarnings: set
> + @return: None (modifies formatter)
> + """
> + full = options.mode == 'full'
> + for category, number in stats.items():
> + # we only want key value pairs where value > 0
> + if number < 1:
> + continue
> +
> + formatter.add_literal_data("NumberOf " + category +
> " ")
> + if category in qawarnings:
> + formatter.push_style("WARN")
> + else:
> + formatter.push_style("BAD")
> + formatter.add_literal_data("%s" % number)
> + formatter.pop_style()
> + formatter.add_line_break()
> + if not dofull:
> + if not full and dofail and category in
> qawarnings:
> + # warnings are considered noise when
> there are failures
> + continue
> + fails_list = fails[category]
> + if not full and len(fails_list) > 12:
> + fails_list = fails_list[:12]
> + for failure in fails_list:
> + formatter.add_literal_data(category
> + " " + failure)
> + formatter.add_line_break()
> +
> def editor_is_executable(editor):
> """
> Given an EDITOR string, validate that it refers to
looks good to me.
--
Brian Dolbec <dolsen>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [gentoo-portage-dev] [PATCH v2] Add --output-style option to repoman
2014-02-11 1:22 ` [gentoo-portage-dev] [PATCH v2] " Chris Reffett
2014-02-11 6:35 ` Brian Dolbec
@ 2014-02-13 8:19 ` Mike Frysinger
2014-02-13 15:42 ` Brian Dolbec
2014-02-13 18:00 ` Alec Warner
1 sibling, 2 replies; 12+ messages in thread
From: Mike Frysinger @ 2014-02-13 8:19 UTC (permalink / raw
To: gentoo-portage-dev; +Cc: Chris Reffett
[-- Attachment #1: Type: text/plain, Size: 1578 bytes --]
On Monday, February 10, 2014 20:22:36 Chris Reffett wrote:
> This patch adds a --output-style option to repoman, which gives the user
> a choice of output formats for the repoman checks. Choices are "default"
> (current style) and "column" (a greppable format), but it should be easy
> to add more. Fixes bug 481584.
i'd expect a proper structured output would make sense to include in the
default set. like JSON. just create a dict and send it to json.dump().
> v2: Fix docstring to be complete and in the standard format, make use of
> default choices in --output-style wrt comments by antarus and dol-sen
erm, i thought the previous docstring was correct. it followed PEP257 while
this new one is like javadoc or something.
> -utilities.format_qa_output(f, stats, fails, dofull, dofail, options,
> qawarnings)
> +if options.output_style == 'column':
> + utilities.format_qa_output_column(f, stats, fails, dofull, dofail,
> options, qawarnings)
> +else:
> + utilities.format_qa_output(f, stats, fails, dofull, dofail, options,
> qawarnings)
use a func pointer instead.
format_outputs = {
'column': utilities.format_qa_output_column,
'default': utilities.format_qa_output,
}
format_output = format_outputs.get(options.output_style,
format_outputs['default'])
format_output(f, stats, fails, dofull, dofail, options, qawarnings)
> + formatter.add_literal_data("NumberOf " + category + " ")
prefer to use % rather than + like so:
'NumberOf %s ' % category
> + formatter.add_literal_data("%s" % number)
str(number)
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [gentoo-portage-dev] [PATCH v2] Add --output-style option to repoman
2014-02-13 8:19 ` Mike Frysinger
@ 2014-02-13 15:42 ` Brian Dolbec
2014-02-13 18:03 ` Alec Warner
` (2 more replies)
2014-02-13 18:00 ` Alec Warner
1 sibling, 3 replies; 12+ messages in thread
From: Brian Dolbec @ 2014-02-13 15:42 UTC (permalink / raw
To: gentoo-portage-dev
[-- Attachment #1: Type: text/plain, Size: 2525 bytes --]
On Thu, 13 Feb 2014 03:19:35 -0500
Mike Frysinger <vapier@gentoo.org> wrote:
> On Monday, February 10, 2014 20:22:36 Chris Reffett wrote:
> > This patch adds a --output-style option to repoman, which gives the
> > user a choice of output formats for the repoman checks. Choices are
> > "default" (current style) and "column" (a greppable format), but it
> > should be easy to add more. Fixes bug 481584.
>
> i'd expect a proper structured output would make sense to include in
> the default set. like JSON. just create a dict and send it to
> json.dump().
He is working on more changes to repoman and the output. So, if you
can, Chris, then do it, add a json option.
>
> > v2: Fix docstring to be complete and in the standard format, make
> > use of default choices in --output-style wrt comments by antarus
> > and dol-sen
>
> erm, i thought the previous docstring was correct. it followed
> PEP257 while this new one is like javadoc or something.
>
It is the existing format that has been around in portage for years.
There is even a page for it:
http://www.gentoo.org/proj/en/portage/doc/policies/docstring-spec.xml
It is also the style that epydoc recognizes.
> > -utilities.format_qa_output(f, stats, fails, dofull, dofail,
> > options, qawarnings)
> > +if options.output_style == 'column':
> > + utilities.format_qa_output_column(f, stats, fails, dofull,
> > dofail, options, qawarnings)
> > +else:
> > + utilities.format_qa_output(f, stats, fails, dofull,
> > dofail, options, qawarnings)
>
> use a func pointer instead.
> format_outputs = {
> 'column': utilities.format_qa_output_column,
> 'default': utilities.format_qa_output,
> }
> format_output = format_outputs.get(options.output_style,
> format_outputs['default'])
> format_output(f, stats, fails, dofull, dofail, options, qawarnings)
>
yeah, make it so. Good spot, Mike
Since Mike was too slow in replying, make another commit to change
it.
> > + formatter.add_literal_data("NumberOf " + category
> > + " ")
>
> prefer to use % rather than + like so:
> 'NumberOf %s ' % category
>
> > + formatter.add_literal_data("%s" % number)
>
well actually, for simple additions like that, string1 + string2, it is
actually faster.
But for multiple additions, %s is much better, faster. Also if the
string is translated, then use %s regardless. That way the %s can be
moved around for the translation.
> str(number)
> -mike
--
Brian Dolbec <dolsen>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 620 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [gentoo-portage-dev] [PATCH v2] Add --output-style option to repoman
2014-02-13 15:42 ` Brian Dolbec
@ 2014-02-13 18:03 ` Alec Warner
2014-02-13 21:02 ` Brian Dolbec
2014-02-14 18:20 ` Chris Reffett
2014-02-19 17:52 ` Chris Reffett
2 siblings, 1 reply; 12+ messages in thread
From: Alec Warner @ 2014-02-13 18:03 UTC (permalink / raw
To: gentoo-portage-dev
[-- Attachment #1: Type: text/plain, Size: 3059 bytes --]
On Thu, Feb 13, 2014 at 7:42 AM, Brian Dolbec <dolsen@gentoo.org> wrote:
> On Thu, 13 Feb 2014 03:19:35 -0500
> Mike Frysinger <vapier@gentoo.org> wrote:
>
> > On Monday, February 10, 2014 20:22:36 Chris Reffett wrote:
> > > This patch adds a --output-style option to repoman, which gives the
> > > user a choice of output formats for the repoman checks. Choices are
> > > "default" (current style) and "column" (a greppable format), but it
> > > should be easy to add more. Fixes bug 481584.
> >
> > i'd expect a proper structured output would make sense to include in
> > the default set. like JSON. just create a dict and send it to
> > json.dump().
>
> He is working on more changes to repoman and the output. So, if you
> can, Chris, then do it, add a json option.
>
>
> >
> > > v2: Fix docstring to be complete and in the standard format, make
> > > use of default choices in --output-style wrt comments by antarus
> > > and dol-sen
> >
> > erm, i thought the previous docstring was correct. it followed
> > PEP257 while this new one is like javadoc or something.
> >
>
> It is the existing format that has been around in portage for years.
> There is even a page for it:
>
> http://www.gentoo.org/proj/en/portage/doc/policies/docstring-spec.xml
>
> It is also the style that epydoc recognizes.
>
> > > -utilities.format_qa_output(f, stats, fails, dofull, dofail,
> > > options, qawarnings)
> > > +if options.output_style == 'column':
> > > + utilities.format_qa_output_column(f, stats, fails, dofull,
> > > dofail, options, qawarnings)
> > > +else:
> > > + utilities.format_qa_output(f, stats, fails, dofull,
> > > dofail, options, qawarnings)
> >
> > use a func pointer instead.
> > format_outputs = {
> > 'column': utilities.format_qa_output_column,
> > 'default': utilities.format_qa_output,
> > }
> > format_output = format_outputs.get(options.output_style,
> > format_outputs['default'])
> > format_output(f, stats, fails, dofull, dofail, options, qawarnings)
> >
>
> yeah, make it so. Good spot, Mike
>
>
> Since Mike was too slow in replying, make another commit to change
> it.
>
> > > + formatter.add_literal_data("NumberOf " + category
> > > + " ")
> >
> > prefer to use % rather than + like so:
> > 'NumberOf %s ' % category
> >
> > > + formatter.add_literal_data("%s" % number)
> >
>
> well actually, for simple additions like that, string1 + string2, it is
> actually faster.
> But for multiple additions, %s is much better, faster. Also if the
> string is translated, then use %s regardless. That way the %s can be
> moved around for the translation.
>
In general we prefer % for readability purposes, not because it is faster.
foo = "Bar" + foo + " " + baz + "," + goat
foo = "Bar %s %s, %s" % (foo, baz, goat)
I think this case could go either way, because even with %, "NumberOf%s" is
not much of an improvement.
The code is littered with the former though, and it makes it really
annoying to read ;)
-A
>
> > str(number)
> > -mike
>
>
>
> --
> Brian Dolbec <dolsen>
>
>
[-- Attachment #2: Type: text/html, Size: 4542 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [gentoo-portage-dev] [PATCH v2] Add --output-style option to repoman
2014-02-13 18:03 ` Alec Warner
@ 2014-02-13 21:02 ` Brian Dolbec
0 siblings, 0 replies; 12+ messages in thread
From: Brian Dolbec @ 2014-02-13 21:02 UTC (permalink / raw
To: gentoo-portage-dev
On Thu, 13 Feb 2014 10:03:50 -0800
Alec Warner <antarus@gentoo.org> wrote:
> On Thu, Feb 13, 2014 at 7:42 AM, Brian Dolbec <dolsen@gentoo.org>
> wrote:
>
> > well actually, for simple additions like that, string1 + string2,
> > it is actually faster.
> > But for multiple additions, %s is much better, faster. Also if the
> > string is translated, then use %s regardless. That way the %s can
> > be moved around for the translation.
> >
>
> In general we prefer % for readability purposes, not because it is
> faster.
>
> foo = "Bar" + foo + " " + baz + "," + goat
>
> foo = "Bar %s %s, %s" % (foo, baz, goat)
>
> I think this case could go either way, because even with %,
> "NumberOf%s" is not much of an improvement.
> The code is littered with the former though, and it makes it really
> annoying to read ;)
>
> -A
Do I have to sick Brian Harring on you ;)
I said for simple string addition... string1 + string2 is faster and
equally readable
Your example goes into the "string %s %s, %s" example where the string
substitution is actually faster. Plus it is a lot more readable.
--
Brian Dolbec <dolsen>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [gentoo-portage-dev] [PATCH v2] Add --output-style option to repoman
2014-02-13 15:42 ` Brian Dolbec
2014-02-13 18:03 ` Alec Warner
@ 2014-02-14 18:20 ` Chris Reffett
2014-02-19 17:52 ` Chris Reffett
2 siblings, 0 replies; 12+ messages in thread
From: Chris Reffett @ 2014-02-14 18:20 UTC (permalink / raw
To: gentoo-portage-dev
On 2/13/2014 10:42 AM, Brian Dolbec wrote:
> On Thu, 13 Feb 2014 03:19:35 -0500
> Mike Frysinger <vapier@gentoo.org> wrote:
>
>> On Monday, February 10, 2014 20:22:36 Chris Reffett wrote:
>>> This patch adds a --output-style option to repoman, which gives the
>>> user a choice of output formats for the repoman checks. Choices are
>>> "default" (current style) and "column" (a greppable format), but it
>>> should be easy to add more. Fixes bug 481584.
>>
>> i'd expect a proper structured output would make sense to include in
>> the default set. like JSON. just create a dict and send it to
>> json.dump().
>
> He is working on more changes to repoman and the output. So, if you
> can, Chris, then do it, add a json option.
>
Sure, I'll take a crack at this.
>
>>
>>> v2: Fix docstring to be complete and in the standard format, make
>>> use of default choices in --output-style wrt comments by antarus
>>> and dol-sen
>>
>> erm, i thought the previous docstring was correct. it followed
>> PEP257 while this new one is like javadoc or something.
>>
>
> It is the existing format that has been around in portage for years.
> There is even a page for it:
>
> http://www.gentoo.org/proj/en/portage/doc/policies/docstring-spec.xml
>
> It is also the style that epydoc recognizes.
>
>>> -utilities.format_qa_output(f, stats, fails, dofull, dofail,
>>> options, qawarnings)
>>> +if options.output_style == 'column':
>>> + utilities.format_qa_output_column(f, stats, fails, dofull,
>>> dofail, options, qawarnings)
>>> +else:
>>> + utilities.format_qa_output(f, stats, fails, dofull,
>>> dofail, options, qawarnings)
>>
>> use a func pointer instead.
>> format_outputs = {
>> 'column': utilities.format_qa_output_column,
>> 'default': utilities.format_qa_output,
>> }
>> format_output = format_outputs.get(options.output_style,
>> format_outputs['default'])
>> format_output(f, stats, fails, dofull, dofail, options, qawarnings)
>>
>
> yeah, make it so. Good spot, Mike
>
Will make this change when I'm back at my devbox (probably Mondayish).
>
> Since Mike was too slow in replying, make another commit to change
> it.
>
>>> + formatter.add_literal_data("NumberOf " + category
>>> + " ")
>>
>> prefer to use % rather than + like so:
>> 'NumberOf %s ' % category
>>
>>> + formatter.add_literal_data("%s" % number)
>>
>
> well actually, for simple additions like that, string1 + string2, it is
> actually faster.
> But for multiple additions, %s is much better, faster. Also if the
> string is translated, then use %s regardless. That way the %s can be
> moved around for the translation.
>
>> str(number)
>> -mike
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [gentoo-portage-dev] [PATCH v2] Add --output-style option to repoman
2014-02-13 15:42 ` Brian Dolbec
2014-02-13 18:03 ` Alec Warner
2014-02-14 18:20 ` Chris Reffett
@ 2014-02-19 17:52 ` Chris Reffett
2 siblings, 0 replies; 12+ messages in thread
From: Chris Reffett @ 2014-02-19 17:52 UTC (permalink / raw
To: gentoo-portage-dev
On 02/13/2014 10:42 AM, Brian Dolbec wrote:
> On Thu, 13 Feb 2014 03:19:35 -0500
> Mike Frysinger <vapier@gentoo.org> wrote:
>
>> On Monday, February 10, 2014 20:22:36 Chris Reffett wrote:
>>> This patch adds a --output-style option to repoman, which gives the
>>> user a choice of output formats for the repoman checks. Choices are
>>> "default" (current style) and "column" (a greppable format), but it
>>> should be easy to add more. Fixes bug 481584.
>>
>> i'd expect a proper structured output would make sense to include in
>> the default set. like JSON. just create a dict and send it to
>> json.dump().
>
> He is working on more changes to repoman and the output. So, if you
> can, Chris, then do it, add a json option.
>
Will do that after my next set of changes to repoman (to be emailed shortly)
>
>>
>>> v2: Fix docstring to be complete and in the standard format, make
>>> use of default choices in --output-style wrt comments by antarus
>>> and dol-sen
>>
>> erm, i thought the previous docstring was correct. it followed
>> PEP257 while this new one is like javadoc or something.
>>
>
> It is the existing format that has been around in portage for years.
> There is even a page for it:
>
> http://www.gentoo.org/proj/en/portage/doc/policies/docstring-spec.xml
>
> It is also the style that epydoc recognizes.
>
>>> -utilities.format_qa_output(f, stats, fails, dofull, dofail,
>>> options, qawarnings)
>>> +if options.output_style == 'column':
>>> + utilities.format_qa_output_column(f, stats, fails, dofull,
>>> dofail, options, qawarnings)
>>> +else:
>>> + utilities.format_qa_output(f, stats, fails, dofull,
>>> dofail, options, qawarnings)
>>
>> use a func pointer instead.
>> format_outputs = {
>> 'column': utilities.format_qa_output_column,
>> 'default': utilities.format_qa_output,
>> }
>> format_output = format_outputs.get(options.output_style,
>> format_outputs['default'])
>> format_output(f, stats, fails, dofull, dofail, options, qawarnings)
>>
>
> yeah, make it so. Good spot, Mike
>
Committed, thanks for the spot.
>
> Since Mike was too slow in replying, make another commit to change
> it.
>
>>> + formatter.add_literal_data("NumberOf " + category
>>> + " ")
>>
>> prefer to use % rather than + like so:
>> 'NumberOf %s ' % category
>>
>>> + formatter.add_literal_data("%s" % number)
>>
>
> well actually, for simple additions like that, string1 + string2, it is
> actually faster.
> But for multiple additions, %s is much better, faster. Also if the
> string is translated, then use %s regardless. That way the %s can be
> moved around for the translation.
>
>> str(number)
>> -mike
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [gentoo-portage-dev] [PATCH v2] Add --output-style option to repoman
2014-02-13 8:19 ` Mike Frysinger
2014-02-13 15:42 ` Brian Dolbec
@ 2014-02-13 18:00 ` Alec Warner
1 sibling, 0 replies; 12+ messages in thread
From: Alec Warner @ 2014-02-13 18:00 UTC (permalink / raw
To: gentoo-portage-dev; +Cc: Chris Reffett
[-- Attachment #1: Type: text/plain, Size: 1951 bytes --]
On Thu, Feb 13, 2014 at 12:19 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Monday, February 10, 2014 20:22:36 Chris Reffett wrote:
> > This patch adds a --output-style option to repoman, which gives the user
> > a choice of output formats for the repoman checks. Choices are "default"
> > (current style) and "column" (a greppable format), but it should be easy
> > to add more. Fixes bug 481584.
>
> i'd expect a proper structured output would make sense to include in the
> default set. like JSON. just create a dict and send it to json.dump().
>
He didn't want to, and I didn't want to make him ;)
>
> > v2: Fix docstring to be complete and in the standard format, make use of
> > default choices in --output-style wrt comments by antarus and dol-sen
>
> erm, i thought the previous docstring was correct. it followed PEP257
> while
> this new one is like javadoc or something.
>
> > -utilities.format_qa_output(f, stats, fails, dofull, dofail, options,
> > qawarnings)
> > +if options.output_style == 'column':
> > + utilities.format_qa_output_column(f, stats, fails, dofull, dofail,
> > options, qawarnings)
> > +else:
> > + utilities.format_qa_output(f, stats, fails, dofull, dofail,
> options,
> > qawarnings)
>
> use a func pointer instead.
> format_outputs = {
> 'column': utilities.format_qa_output_column,
> 'default': utilities.format_qa_output,
> }
> format_output = format_outputs.get(options.output_style,
> format_outputs['default'])
> format_output(f, stats, fails, dofull, dofail, options, qawarnings)
>
> > + formatter.add_literal_data("NumberOf " + category + " ")
>
> prefer to use % rather than + like so:
> 'NumberOf %s ' % category
>
> > + formatter.add_literal_data("%s" % number)
>
> str(number)
>
That is the definition of %s, no explicit str() is necessary.
http://docs.python.org/2/library/stdtypes.html#string-formatting-operations
> -mike
[-- Attachment #2: Type: text/html, Size: 3325 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread