threads / patch / 54213

v2blame.c: replace instance of !oidcmp for oideq

Subject: [PATCH v2] blame.c: replace instance of !oidcmp for oideq

## tl;dr

6 messages between Sep 8, 2020 and Sep 9, 2020. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Edmundo Carmona Antoranz· Sep 8, 2020, 21:10 UTC · lore

0906ac2b54b introduced a call to oidcmp() that should be replaced by oideq() (the latter introduced in 14438c4).

Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>
---
 blame.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to blame.c +2 −2
diff --git a/blame.c b/blame.c
index da7e28800e..a8abe86ae4 100644
--- a/blame.c
+++ b/blame.c
@@ -1352,8 +1352,8 @@ static struct blame_origin *find_origin(struct repository *r,
 	else {
 		int compute_diff = 1;
 		if (origin->commit->parents &&
-		    !oidcmp(&parent->object.oid,
-			    &origin->commit->parents->item->object.oid))
+		    oideq(&parent->object.oid,
+			  &origin->commit->parents->item->object.oid))
 			compute_diff = maybe_changed_path(r, origin, bd);
 
 		if (compute_diff)
-- 
2.28.0
Derrick Stolee· Sep 9, 2020, 12:39 UTC · re: Edmundo Carmona Antoranz · lore

Re: [PATCH v2] blame.c: replace instance of !oidcmp for oideq

On 9/8/2020 5:10 PM, Edmundo Carmona Antoranz wrote:
> 0906ac2b54b introduced a call to oidcmp() that should be
> replaced by oideq() (the latter introduced in 14438c4).

Short-OIDs on their own are not enough. Please use the format "abbreviated hash (subject, date)" as mentioned in my earlier reply:

0906ac2b (blame: use changed-path Bloom filters, 2020-04-16)
14438c4 (introduce hasheq() and oideq(), 2018-08-28)

See Documentation/SubmittingPatches [1] for more information on this.

[1] https://github.com/git/git/blob/3a238e539bcdfe3f9eb5010fd218640c1b499f7a/Documentation/SubmittingPatches#L144-L165

Thanks, -Stolee

Edmundo Carmona Antoranz· Sep 9, 2020, 13:10 UTC · re: Derrick Stolee · lore

Re: [PATCH v2] blame.c: replace instance of !oidcmp for oideq

On Wed, Sep 9, 2020 at 6:39 AM Derrick Stolee <stolee@gmail.com> wrote:
> Short-OIDs on their own are not enough. Please use the
> format "abbreviated hash (subject, date)" as mentioned
> in my earlier reply:
>
You got it. Thanks!
Junio C Hamano· Sep 9, 2020, 18:35 UTC · re: Derrick Stolee · lore

Re: [PATCH v2] blame.c: replace instance of !oidcmp for oideq

Derrick Stolee <stolee@gmail.com> writes:
Show 16 quoted lines
> On 9/8/2020 5:10 PM, Edmundo Carmona Antoranz wrote:
>> 0906ac2b54b introduced a call to oidcmp() that should be
>> replaced by oideq() (the latter introduced in 14438c4).
>
> Short-OIDs on their own are not enough. Please use the
> format "abbreviated hash (subject, date)" as mentioned
> in my earlier reply:
>
> 0906ac2b (blame: use changed-path Bloom filters, 2020-04-16)
>
> 14438c4 (introduce hasheq() and oideq(), 2018-08-28)
>
> See Documentation/SubmittingPatches [1] for more information
> on this.
>
> [1] https://github.com/git/git/blob/3a238e539bcdfe3f9eb5010fd218640c1b499f7a/Documentation/SubmittingPatches#L144-L165
Thanks for saving me from having to say it ;-)
I've reworded these while queuing.  See below.
Note that I toned "should be replaced" down to "should have been".

I'd consider this kind of change falls into the "Yes, it would have been better if it were written like so from the beginning, but it is dubious that it is worth the engineering time and effort to go back and change it spending reviewers' bandwidth" category. I'll still take this patch as it is a contribution from a new contributor, though (it's like reviewing and queuing microproject---the cost in such cases is not attributed to code churning alone, but it also benefits onboarding).

Thanks.
-- >8 --
From: Edmundo Carmona Antoranz <eantoranz@gmail.com>
Date: Tue, 8 Sep 2020 15:10:53 -0600
Subject: [PATCH] blame.c: replace instance of !oidcmp for oideq

0906ac2b (blame: use changed-path Bloom filters, 2020-04-16) introduced a call to oidcmp() that should have been oideq(), which was introduced in 14438c44 (introduce hasheq() and oideq(), 2018-08-28).

Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 blame.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to blame.c +2 −2
diff --git a/blame.c b/blame.c
index da7e28800e..a8abe86ae4 100644
--- a/blame.c
+++ b/blame.c
@@ -1352,8 +1352,8 @@ static struct blame_origin *find_origin(struct repository *r,
 	else {
 		int compute_diff = 1;
 		if (origin->commit->parents &&
-		    !oidcmp(&parent->object.oid,
-			    &origin->commit->parents->item->object.oid))
+		    oideq(&parent->object.oid,
+			  &origin->commit->parents->item->object.oid))
 			compute_diff = maybe_changed_path(r, origin, bd);
 
 		if (compute_diff)
-- 
2.28.0-558-g7a0184fd7b
Edmundo Carmona Antoranz· Sep 9, 2020, 19:13 UTC · re: Junio C Hamano · lore

Re: [PATCH v2] blame.c: replace instance of !oidcmp for oideq

On Wed, Sep 9, 2020 at 12:36 PM Junio C Hamano <gitster@pobox.com> wrote:
> I'll still
> take this patch as it is a contribution from a new contributor,

I don't know why you say I'm a new contributor. I would have sworn you have my picture on a wall with something written like: do not take patches from this guy. ;-) It's good to at least know that I haven't messed up so badly that I'm in Junio's name's cache.

>
> Thanks.

Anytime! And thanks for the help from Derrick. Very kind for helping me out with the baby steps.

Junio C Hamano· Sep 9, 2020, 20:08 UTC · re: Edmundo Carmona Antoranz · lore

Re: [PATCH v2] blame.c: replace instance of !oidcmp for oideq

Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:
Show 9 quoted lines
> On Wed, Sep 9, 2020 at 12:36 PM Junio C Hamano <gitster@pobox.com> wrote:
>> I'll still
>> take this patch as it is a contribution from a new contributor,
>
> I don't know why you say I'm a new contributor. I would have sworn you
> have my picture
> on a wall with something written like: do not take patches from this guy. ;-)
> It's good to at least know that I haven't messed up so badly that I'm
> in Junio's name's cache.

Sorry, it appears that two patches from late 2015 weren't enough to retain your name kept in cache X-<.

← back to recent threads