Re: [libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive

On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote:
On 10/10/19 12:43 AM, John Snow wrote:
It's an old compatibility shim that just delegates to ide-cd or ide-hd. I'd like to refactor these some day, and getting rid of the super-object will make that easier.
Either way, we don't need this.
Libvirt-checked-by: Peter Krempa <pkrempa@redhat.com>
Peter made a comment regarding Laszlo's Regression-tested-by tag:
[...] nobody else is using this convention (there are exactly 0 instances of "Regression-tested-by" in the project git log as far as I can see), and so in practice people reading the commits won't really know what you meant by it. Everybody else on the project uses "Tested-by" to mean either of the two cases you describe above, without distinction...
It probably applies to 'Libvirt-checked-by' too.
I certainly didn't test it. So feel free to drop that line altogether.

On 10/10/19 1:26 PM, Peter Krempa wrote:
On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote:
On 10/10/19 12:43 AM, John Snow wrote:
It's an old compatibility shim that just delegates to ide-cd or ide-hd. I'd like to refactor these some day, and getting rid of the super-object will make that easier.
Either way, we don't need this.
Libvirt-checked-by: Peter Krempa <pkrempa@redhat.com>
Peter made a comment regarding Laszlo's Regression-tested-by tag:
[...] nobody else is using this convention (there are exactly 0 instances of "Regression-tested-by" in the project git log as far as I can see), and so in practice people reading the commits won't really know what you meant by it. Everybody else on the project uses "Tested-by" to mean either of the two cases you describe above, without distinction...
It probably applies to 'Libvirt-checked-by' too.
I certainly didn't test it. So feel free to drop that line altogether.
But you reviewed it, can we use your 'Reviewed-by' instead?

On Thu, Oct 10, 2019 at 13:42:26 +0200, Philippe Mathieu-Daudé wrote:
On 10/10/19 1:26 PM, Peter Krempa wrote:
On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote:
On 10/10/19 12:43 AM, John Snow wrote:
It's an old compatibility shim that just delegates to ide-cd or ide-hd. I'd like to refactor these some day, and getting rid of the super-object will make that easier.
Either way, we don't need this.
Libvirt-checked-by: Peter Krempa <pkrempa@redhat.com>
Peter made a comment regarding Laszlo's Regression-tested-by tag:
[...] nobody else is using this convention (there are exactly 0 instances of "Regression-tested-by" in the project git log as far as I can see), and so in practice people reading the commits won't really know what you meant by it. Everybody else on the project uses "Tested-by" to mean either of the two cases you describe above, without distinction...
It probably applies to 'Libvirt-checked-by' too.
I certainly didn't test it. So feel free to drop that line altogether.
But you reviewed it, can we use your 'Reviewed-by' instead?
To be honest, I didn't really review the code nor the documentation. I actually reviewed only the idea itself in the context of integration with libvirt and that's why I didn't go for 'Reviewed-by:'. The gist of the citation above is that we should stick to well known tags with their well known meanings and I think that considering this a 'review' would be a stretch of the definiton.

On 10/10/19 7:54 AM, Peter Krempa wrote:
On Thu, Oct 10, 2019 at 13:42:26 +0200, Philippe Mathieu-Daudé wrote:
On 10/10/19 1:26 PM, Peter Krempa wrote:
On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote:
On 10/10/19 12:43 AM, John Snow wrote:
It's an old compatibility shim that just delegates to ide-cd or ide-hd. I'd like to refactor these some day, and getting rid of the super-object will make that easier.
Either way, we don't need this.
Libvirt-checked-by: Peter Krempa <pkrempa@redhat.com>
Peter made a comment regarding Laszlo's Regression-tested-by tag:
[...] nobody else is using this convention (there are exactly 0 instances of "Regression-tested-by" in the project git log as far as I can see), and so in practice people reading the commits won't really know what you meant by it. Everybody else on the project uses "Tested-by" to mean either of the two cases you describe above, without distinction...
It probably applies to 'Libvirt-checked-by' too.
I certainly didn't test it. So feel free to drop that line altogether.
But you reviewed it, can we use your 'Reviewed-by' instead?
To be honest, I didn't really review the code nor the documentation. I actually reviewed only the idea itself in the context of integration with libvirt and that's why I didn't go for 'Reviewed-by:'.
The gist of the citation above is that we should stick to well known tags with their well known meanings and I think that considering this a 'review' would be a stretch of the definiton.
I wasn't aware that PMM wanted to avoid non-standard tags; I consider them to be for human use, but I can change that behavior. Peter, I'll change it to an ACK (as suggested by Kevin) is that's OK by you. --js

On Thu, Oct 10, 2019 at 14:08:12 -0400, John Snow wrote:
On 10/10/19 7:54 AM, Peter Krempa wrote:
On Thu, Oct 10, 2019 at 13:42:26 +0200, Philippe Mathieu-Daudé wrote:
On 10/10/19 1:26 PM, Peter Krempa wrote:
[...]
To be honest, I didn't really review the code nor the documentation. I actually reviewed only the idea itself in the context of integration with libvirt and that's why I didn't go for 'Reviewed-by:'.
The gist of the citation above is that we should stick to well known tags with their well known meanings and I think that considering this a 'review' would be a stretch of the definiton.
I wasn't aware that PMM wanted to avoid non-standard tags; I consider them to be for human use, but I can change that behavior.
Peter, I'll change it to an ACK (as suggested by Kevin) is that's OK by you.
Sure! I'll spell it out even for compliance: ACKed-by: Peter Krempa <pkrempa@redhat.com>

On 10/11/19 5:12 AM, Peter Krempa wrote:
On Thu, Oct 10, 2019 at 14:08:12 -0400, John Snow wrote:
On 10/10/19 7:54 AM, Peter Krempa wrote:
On Thu, Oct 10, 2019 at 13:42:26 +0200, Philippe Mathieu-Daudé wrote:
On 10/10/19 1:26 PM, Peter Krempa wrote:
[...]
To be honest, I didn't really review the code nor the documentation. I actually reviewed only the idea itself in the context of integration with libvirt and that's why I didn't go for 'Reviewed-by:'.
The gist of the citation above is that we should stick to well known tags with their well known meanings and I think that considering this a 'review' would be a stretch of the definiton.
I wasn't aware that PMM wanted to avoid non-standard tags; I consider them to be for human use, but I can change that behavior.
Peter, I'll change it to an ACK (as suggested by Kevin) is that's OK by you.
Sure! I'll spell it out even for compliance:
ACKed-by: Peter Krempa <pkrempa@redhat.com>
Thanks, applied to my IDE tree: https://github.com/jnsnow/qemu/commits/ide https://github.com/jnsnow/qemu.git --js
participants (3)
-
John Snow
-
Peter Krempa
-
Philippe Mathieu-Daudé