Hi Simon,
On Mon, Mar 13, 2017 at 02:36:28PM +0100, Simon Horman wrote:
> Hi Willy,
>
> this patchset seems to have stalled.
> I'd like to find a way to revive it.
Just in time :-) I was about to ask if you had any news, but apparently
I was the one stalling here.
> > > Thanks for this. I'm seeing in stats_emit_json_field_tags() that
> > > you have to emit the names of the various types, scopes, etc...
> > > I think this is the reason why you mention in patch 1 that it needs
> > > to be updated if the structure evolves. Probably that we should put
> > > these fields in an array declared just next to the enums. This way
> > > the declaration will be a bit more centralized.
>
> I see how that may make things more centralised in theory.
> But in practice I wonder how it might work.
>
> enum field_origin, for example, is declared in stats.h.
>
> However, if I put something like the following into the same file
> then it will be defined multiple times as the header file is included
> in the source for multiple objects.
>
> const char *field_origin_long_name[] = {
> [FO_METRIC] = "Metric",
> ...
> };
>
> Of course I can move the above into a .c file but then it is no
> longer centralised with the enum it relates to.
That's a very good point, I agree that having the enums at one place and
the declaration at another is not always cool. Let's take it as-is and
see if anyone comes with a better idea later.
> > > > Some areas for possible discussion:
> > > > * Use of STAT_STARTED in first patch
> > > > * Possible automatic generation of (part) of schema in 2nd patch
> > > > * Improved documentation
> > >
> > > For now I don't see anything there which needs further discussion, and
> > > nobody commented on your patches either, possibly indicating you're on
> > > the right track. If you want I can merge this series, it will be easier
> > > for you to update it later using incremental patches.
> > >
> > > > Some discussion of the size of JSON output is included as an appendix
> > > > to the changelog of the first patch.
> > >
> > > I'd prefer to integrate this with your commit message because it's quite
> > > useful as-is.
> > >
> > > Just let me know if you want the series to get merged or if you prefer
> > > to respin it.
> >
> > I'd prefer if you merged the series as-is
> > and I then provided incremental updates.
>
> FWIW, I believe the series still applies cleanly on master.
OK, I'll check ASAP.
Thanks!
Willy
