[PATCH 001/103] virJSONValueObjectAddVArgs: Add 'k' convertor for formatting non-negative integers

[Ján Tomko]

On a Friday in 2021, Kristina Hanicova wrote:
>On Thu, Oct 7, 2021 at 6:43 PM Ján Tomko <jtomko at redhat.com> wrote:
>
>> On a Thursday in 2021, Peter Krempa wrote:
>> >In many cases we use a signed value, but use the sign to note that it
>> >was not assigned. For converting to JSON objects it will be handy to
>> >have possibility to do this automatically.
>> >
>> >Signed-off-by: Peter Krempa <pkrempa at redhat.com>
>> >---
>> > src/util/virjson.c | 7 ++++++-
>> > 1 file changed, 6 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/src/util/virjson.c b/src/util/virjson.c
>> >index 9adcea4fff..70ea71b505 100644
>> >--- a/src/util/virjson.c
>> >+++ b/src/util/virjson.c
>> >@@ -115,6 +115,7 @@ virJSONValueGetType(const virJSONValue *value)
>> >  *
>> >  * i: signed integer value
>> >  * j: signed integer value, error if negative
>> >+ * k: signed integer value, omitted if negative
>> >  * z: signed integer value, omitted if zero
>> >  * y: signed integer value, omitted if zero, error if negative
>> >  *
>> >@@ -189,6 +190,7 @@ virJSONValueObjectAddVArgs(virJSONValue *obj,
>> >
>> >         case 'z':
>> >         case 'y':
>> >+        case 'k':
>> >         case 'j':
>> >         case 'i': {
>> >             int val = va_arg(args, int);
>> >@@ -200,7 +202,10 @@ virJSONValueObjectAddVArgs(virJSONValue *obj,
>> >                 return -1;
>> >             }
>> >
>> >-            if (!val && (type == 'z' || type == 'y'))
>> >+            if (val == 0 && (type == 'z' || type == 'y'))
>> >+                continue;
>> >+
>>
>> Please split out this cosmetic style change.
>>
>
>Please don't.
>

Why not?

It is unrelated to the goal of introducing the 'k' parser
and having it here distracts from that goal.

Writing a new commit might cost a few seconds of the author's time,
but it will save time for the readers of such commit. And you can
expect a commit to be read more times (even by the author a few years
into the future, after they long forgot about writing it), while it is
only written once.

There might be some projects (I think linux kernel does this) that do
not accept commits with purely  whitespace/style changes, but we do allow
them in libvirt and I'm much happier to review them, since they make the
actual functional changes more obvious. (And they make the review of a 100-patch
series possible).

I like this blog post danpb wrote on the topic:
https://www.berrange.com/posts/2012/06/27/thoughts-on-improving-openstack-git-commit-practicehistory/

Jano

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20211008/6d34c547/attachment-0001.sig>