Re: [Qemu-devel] [PATCH 15/30] qmp-commands: move 'query-migrate-parameters' doc to schema

[Marc-André Lureau]

Hi

----- Original Message -----
> Marc-André Lureau <address@hidden> writes:
> 
> > Hi
> >
> > ----- Original Message -----
> >> Marc-André Lureau <address@hidden> writes:
> >> 
> >> > Hi
> >> >
> >> > ----- Original Message -----
> >> >> On 09/13/2016 08:01 AM, Marc-André Lureau wrote:
> >> >> > Signed-off-by: Marc-André Lureau <address@hidden>
> >> >> > ---
> >> >> >  docs/qmp-commands.txt | 29 -----------------------------
> >> >> >  qapi-schema.json      | 13 +++++++++++++
> >> >> >  2 files changed, 13 insertions(+), 29 deletions(-)
> >> >> > 
> >> >> 
> >> >> > +++ b/qapi-schema.json
> >> >> > @@ -1011,6 +1011,19 @@
> >> >> >  # Returns: @MigrationParameters
> >> >> >  #
> >> >> >  # Since: 2.4
> >> >> > +#
> >> >> > +# Example:
> >> >> > +#
> >> >> > +# -> { "execute": "query-migrate-parameters" }
> >> >> > +# <- { "return": {
> >> >> > +#          "decompress-threads": 2,
> >> >> > +#          "cpu-throttle-increment": 10,
> >> >> > +#          "compress-threads": 8,
> >> >> > +#          "compress-level": 1,
> >> >> > +#          "cpu-throttle-initial": 20
> >> >> > +#       }
> >> >> > +#    }
> >> >> > +#
> >> >> >  ##
> >> >> >  { 'command': 'query-migrate-parameters',
> >> >> >    'returns': 'MigrationParameters' }
> >> >> 
> >> >> The example lacks 'cpu-throttle-increment', 'tls-creds', and
> >> >> 'tls-hostname'; do we want to take this opportunity to touch it up?
> >> >
> >> > I suggest to put a [...] in the returned example, as this example could
> >> > grow again, and there isn't much to learn from that query.
> >> >  
> >> >> Meanwhile, I have a series that touches this code, and will obviously
> >> >> create a merge conflict for whoever gets in second:
> >> >> https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg01946.html
> >> >
> >> > Yes, the more we wait to review the series, the more conflicts we will
> >> > get.
> >> > There is still over 100 patches to go, I'll send the next 30.
> >> 
> >> We suggested restructuring the series, and you liked the idea with the
> >> alternative step (3b), not (3a).  Would it make sense to repost the
> >> beginning of the multi-part monster in that form before moving on to the
> >> next part?
> >
> >  3. Merge qmp-commands.txt into QAPI schema comments, step by step
> >
> >    (b) If you delete qmp-commands.txt section as you cover them in the
> >    QAPI schema, command documentation regresses temporarily.  Tolerable,
> >    but needs to be explained in commit messages.  Your choice.
> >
> > Isn't that what this series is doing? it moves the remaining doc from
> > qmp-commands.txt to the schema.
> 
> Misunderstanding?  Step (3b) is one step of a reordered series.  Let me
> repeat the order I proposed:
> 
> 1. Fix existing issues in QAPI schema comments
> 
> 2. Generate documentation from it (not a replacement for
>    qmp-commands.txt, yet)
> 
> 3. Merge qmp-commands.txt into QAPI schema comments, step by step
> 
>    (a) If you only update the QAPI schema comments, qmp-commands.txt
>    stays intact throughout this work.
> 
>    (b) If you delete qmp-commands.txt section as you cover them in the
>    QAPI schema, command documentation regresses temporarily.  Tolerable,
>    but needs to be explained in commit messages.  Your choice.
> 
> 4. Generated documentation now contains everything qmp-commands.txt
>    contains; delete qmp-commands.txt
> 
> This way, the first part contains everything that's really interesting:
> step 1, 2 and some of 3a or 3b, depending on which alternative you pick.
> The remaining parts are just more of 3a or 3b, plus the trivial step 4
> in the last one.
> 
> I proposed this to get the interesting review of step 2 out of the way
> early, and before we tire ourselves out on the not-so-interesting but
> necessary review of step 3.
> 

It would have been easier to keep the discussion on the original thread. I 
disagreed with this plan:

 Generating the documentation before the end of 3(b) will also lead to 
temporarily incomplete generated doc, and will conflict with existing 
qmp-commands.txt.
 That's why I think the best solution is to go through 3(b) now, collect the 
move in a branch and push it in one go when qmp-commands.txt is empty and the 
doc is generated.