| Message ID | 20160913130209.695-16-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 09/13/2016 08:01 AM, Marc-André Lureau wrote: > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > 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? 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 At any rate, 11-15 are Reviewed-by: Eric Blake <eblake@redhat.com>
Hi ----- Original Message ----- > On 09/13/2016 08:01 AM, Marc-André Lureau wrote: > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > --- > > 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. > At any rate, 11-15 are > Reviewed-by: Eric Blake <eblake@redhat.com> thanks
Marc-André Lureau <mlureau@redhat.com> writes: > Hi > > ----- Original Message ----- >> On 09/13/2016 08:01 AM, Marc-André Lureau wrote: >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> > --- >> > 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? [...]
Hi ----- Original Message ----- > Marc-André Lureau <mlureau@redhat.com> writes: > > > Hi > > > > ----- Original Message ----- > >> On 09/13/2016 08:01 AM, Marc-André Lureau wrote: > >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > >> > --- > >> > 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.
Marc-André Lureau <mlureau@redhat.com> writes: > Hi > > ----- Original Message ----- >> Marc-André Lureau <mlureau@redhat.com> writes: >> >> > Hi >> > >> > ----- Original Message ----- >> >> On 09/13/2016 08:01 AM, Marc-André Lureau wrote: >> >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> >> > --- >> >> > 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.
Hi ----- Original Message ----- > Marc-André Lureau <mlureau@redhat.com> writes: > > > Hi > > > > ----- Original Message ----- > >> Marc-André Lureau <mlureau@redhat.com> writes: > >> > >> > Hi > >> > > >> > ----- Original Message ----- > >> >> On 09/13/2016 08:01 AM, Marc-André Lureau wrote: > >> >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > >> >> > --- > >> >> > 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.
Marc-André Lureau <mlureau@redhat.com> writes: > Hi > > ----- Original Message ----- >> Marc-André Lureau <mlureau@redhat.com> writes: >> >> > Hi >> > >> > ----- Original Message ----- >> >> Marc-André Lureau <mlureau@redhat.com> writes: >> >> >> >> > Hi >> >> > >> >> > ----- Original Message ----- >> >> >> On 09/13/2016 08:01 AM, Marc-André Lureau wrote: >> >> >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> >> >> > --- >> >> >> > 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. Having incomplete new documentation (getting less incomplete in each commit) conflict with incomplete old documentation (getting more incomplete) is no worse than having old documentation getting more incomplete. In both cases, we go through an intermediate state with flawed documentation. > 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. If you prefer to do it this way, I suggest to post everything at once, because I won't bother reviewing any of step 3 before step 2 for the reasons I explained above. I don't want to see review of the step 2 invalidate all our review work on step 3.
Hi ----- Original Message ----- > Marc-André Lureau <mlureau@redhat.com> writes: > > > Hi > > > > ----- Original Message ----- > >> Marc-André Lureau <mlureau@redhat.com> writes: > >> > >> > Hi > >> > > >> > ----- Original Message ----- > >> >> Marc-André Lureau <mlureau@redhat.com> writes: > >> >> > >> >> > Hi > >> >> > > >> >> > ----- Original Message ----- > >> >> >> On 09/13/2016 08:01 AM, Marc-André Lureau wrote: > >> >> >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > >> >> >> > --- > >> >> >> > 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. > > Having incomplete new documentation (getting less incomplete in each > commit) conflict with incomplete old documentation (getting more > incomplete) is no worse than having old documentation getting more > incomplete. In both cases, we go through an intermediate state with > flawed documentation. Not if we merge the move in one go, as I propose. Don't you think it's cleaner if we first move the documentation in one place before we generate yet another incomplete doc? > > > 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. > > If you prefer to do it this way, I suggest to post everything at once, > because I won't bother reviewing any of step 3 before step 2 for the > reasons I explained above. I don't want to see review of the step 2 > invalidate all our review work on step 3. I fail to see how generating the doc could invalidate moving the documentation in the schema. Indeed, we may want to tweak the documentation style for some reason, but that's irrelevant for now, all we should focus on is having the doc in one place before doing further work with it.
On 09/22/2016 06:54 AM, Marc-André Lureau wrote: >> 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. >> > > 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. Incomplete generated doc is fine, if the commit message calls it out that upcoming patches will fix the gaps (after all, that's what 3(b) says to do). And I tend to agree with Markus that reviewing the generator first, rather than after we've burned ourself out looking over lots of individual commands, will give us a chance to make sure the generator is sane (and may even have some minor impacts to HOW we move documentation over, if we end up tweaking the .json files to make the generator easier to manage). > 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. As it is, I already made comments about the first batch where you were mixing in fixes (step 1) with doc motion, but failed to mention it in the commit message. And where Markus has already argued that making those fixes on their own as a prerequisite is a bit cleaner than forcing the review of step 3 to be more complex.
Marc-André Lureau <mlureau@redhat.com> writes: > Hi > > ----- Original Message ----- >> Marc-André Lureau <mlureau@redhat.com> writes: >> >> > Hi >> > >> > ----- Original Message ----- >> >> Marc-André Lureau <mlureau@redhat.com> writes: >> >> >> >> > Hi >> >> > >> >> > ----- Original Message ----- >> >> >> Marc-André Lureau <mlureau@redhat.com> writes: >> >> >> >> >> >> > Hi >> >> >> > >> >> >> > ----- Original Message ----- >> >> >> >> On 09/13/2016 08:01 AM, Marc-André Lureau wrote: >> >> >> >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> [...] >> >> >> >> 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. >> >> Having incomplete new documentation (getting less incomplete in each >> commit) conflict with incomplete old documentation (getting more >> incomplete) is no worse than having old documentation getting more >> incomplete. In both cases, we go through an intermediate state with >> flawed documentation. > > Not if we merge the move in one go, as I propose. > > Don't you think it's cleaner if we first move the documentation in one place before we generate yet another incomplete doc? At this time, I'm 100% focused on ease of review and saving your and your reviewers' time. Once we got review under control, I'm happy to talk about whether another order or some squashing makes more sense for the permanent Git record. >> > 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. >> >> If you prefer to do it this way, I suggest to post everything at once, >> because I won't bother reviewing any of step 3 before step 2 for the >> reasons I explained above. I don't want to see review of the step 2 >> invalidate all our review work on step 3. > > I fail to see how generating the doc could invalidate moving the > documentation in the schema. > > Indeed, we may want to tweak the documentation style for some reason, > but that's irrelevant for now, all we should focus on is having the > doc in one place before doing further work with it. Arguing further about the perfect order feels like a waste of time. You're free to post this in the order you feel is right. If it differs from the order that I feel is right for my review, then please post the whole thing, so I can review it effectively.
Hi ----- Original Message ----- > Arguing further about the perfect order feels like a waste of time. > > You're free to post this in the order you feel is right. If it differs > from the order that I feel is right for my review, then please post the > whole thing, so I can review it effectively. Since only you and Eric are actually reviewing this, and you both prefer to review the parser/generator first, I sent it yesterday: http://patchew.org/QEMU/20160922155808.8504-1-marcandre.lureau%40redhat.com/
diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt index fc6f054..d96dd27 100644 --- a/docs/qmp-commands.txt +++ b/docs/qmp-commands.txt @@ -2441,35 +2441,6 @@ Example: ] } -query-migrate-parameters ------------------------- - -Query current migration parameters - -- "parameters": migration parameters value - - "compress-level" : compression level value (json-int) - - "compress-threads" : compression thread count value (json-int) - - "decompress-threads" : decompression thread count value (json-int) - - "cpu-throttle-initial" : initial percentage of time guest cpus are - throttled (json-int) - - "cpu-throttle-increment" : throttle increasing percentage for - auto-converge (json-int) - -Arguments: - -Example: - --> { "execute": "query-migrate-parameters" } -<- { - "return": { - "decompress-threads": 2, - "cpu-throttle-increment": 10, - "compress-threads": 8, - "compress-level": 1, - "cpu-throttle-initial": 20 - } - } - query-balloon ------------- diff --git a/qapi-schema.json b/qapi-schema.json index df83457..e57f223 100644 --- a/qapi-schema.json +++ 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' }
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- docs/qmp-commands.txt | 29 ----------------------------- qapi-schema.json | 13 +++++++++++++ 2 files changed, 13 insertions(+), 29 deletions(-)