From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH v2] support/script/pkg-stats: add support for json output
Date: Thu, 11 Jul 2019 17:51:04 +0200 [thread overview]
Message-ID: <20190711175104.45c6f597@windsurf> (raw)
In-Reply-To: <20190711092801.6314-1-victor.huesca@bootlin.com>
Hello Victor,
Thanks for this work!
On Thu, 11 Jul 2019 11:28:02 +0200
Victor Huesca <victor.huesca@bootlin.com> wrote:
> Pkg-stats is a great script that get a lot of intersting infos from
Typo: interesting
> buildroot packages. Unfortunatly it is currently designed to output a
Typo: unfortunately
> static HTML page only. While html is great for human to read, it is a
> pain to parse for scripts.
>
> This patch provide a new option to output a JSON file in addition to the
provide -> provides
> HTML one. This allow other scripts to use all the usefull informations
allow -> allows
informations -> information
> computed by pkg-stats.
>
> Please note that the old 'output' option has been renamed to 'html'
> to distinguish from the new 'json' option. Also the 'npackages' and
> 'packages' now uses the argparse `mutualy_exclusive` group.
uses -> use
mutually_exclusive
>
> Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>
I think ideally this patch should have been split into two:
- One migrating the options to a better argparse usage
- A second one adding the JSON output support itself.
> +def dump_json(packages, stats, output):
> + excluded_fields = ['url_worker', 'name']
> + pkgs = {
> + pkg.name: {
> + k: v
> + for k, v in pkg.__dict__.items()
> + if k not in excluded_fields
> + } for pkg in packages
> + }
> + statistics = {
> + k: v
> + for k, v in stats.items()
> + if not k.startswith('infra-')
> + }
> + statistics['infra'] = {k[6:]: v for k, v in stats.items() if k.startswith('infra-')}
Perhaps a short comment here would be useful to explain what's going on
with the "infra" key in the statistics dict.
> + o = subprocess.check_output(['git', 'log', 'master', '-n', '1', '--pretty=format:%H'])
> + final = {'packages': pkgs,
> + 'stats': statistics,
> + 'commit': o.splitlines()[0],
> + 'date': str(datetime.datetime.utcnow())}
For both the git commit and date, I find it a bit odd that we need to
retrieve it separately once for the HTML output and once for the JSON
output. I think it would make more sense if the calling function of
dump_html() and dump_json() was getting those two values (Git commit id
and date), and pass them as argument to dump_html() and dump_json().
Besides these comments, it looks good to me!
Thanks again,
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
prev parent reply other threads:[~2019-07-11 15:51 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-11 9:28 [Buildroot] [PATCH v2] support/script/pkg-stats: add support for json output Victor Huesca
2019-07-11 15:51 ` Thomas Petazzoni [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190711175104.45c6f597@windsurf \
--to=thomas.petazzoni@bootlin.com \
--cc=buildroot@busybox.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.