git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH v2] blame.c: replace instance of !oidcmp for oideq
@ 2020-09-08 21:10 Edmundo Carmona Antoranz
  2020-09-09 12:39 ` Derrick Stolee
  0 siblings, 1 reply; 6+ messages in thread
From: Edmundo Carmona Antoranz @ 2020-09-08 21:10 UTC (permalink / raw)
  To: dstolee, git, peff; +Cc: Edmundo Carmona Antoranz

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(-)

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] blame.c: replace instance of !oidcmp for oideq
  2020-09-08 21:10 [PATCH v2] blame.c: replace instance of !oidcmp for oideq Edmundo Carmona Antoranz
@ 2020-09-09 12:39 ` Derrick Stolee
  2020-09-09 13:10   ` Edmundo Carmona Antoranz
  2020-09-09 18:35   ` Junio C Hamano
  0 siblings, 2 replies; 6+ messages in thread
From: Derrick Stolee @ 2020-09-09 12:39 UTC (permalink / raw)
  To: Edmundo Carmona Antoranz, dstolee, git, peff

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] blame.c: replace instance of !oidcmp for oideq
  2020-09-09 12:39 ` Derrick Stolee
@ 2020-09-09 13:10   ` Edmundo Carmona Antoranz
  2020-09-09 18:35   ` Junio C Hamano
  1 sibling, 0 replies; 6+ messages in thread
From: Edmundo Carmona Antoranz @ 2020-09-09 13:10 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: dstolee, Git List, Jeff King

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!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] blame.c: replace instance of !oidcmp for oideq
  2020-09-09 12:39 ` Derrick Stolee
  2020-09-09 13:10   ` Edmundo Carmona Antoranz
@ 2020-09-09 18:35   ` Junio C Hamano
  2020-09-09 19:13     ` Edmundo Carmona Antoranz
  1 sibling, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2020-09-09 18:35 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: Edmundo Carmona Antoranz, dstolee, git, peff

Derrick Stolee <stolee@gmail.com> writes:

> 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(-)

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] blame.c: replace instance of !oidcmp for oideq
  2020-09-09 18:35   ` Junio C Hamano
@ 2020-09-09 19:13     ` Edmundo Carmona Antoranz
  2020-09-09 20:08       ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Edmundo Carmona Antoranz @ 2020-09-09 19:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Derrick Stolee, dstolee, Git List, Jeff King

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] blame.c: replace instance of !oidcmp for oideq
  2020-09-09 19:13     ` Edmundo Carmona Antoranz
@ 2020-09-09 20:08       ` Junio C Hamano
  0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2020-09-09 20:08 UTC (permalink / raw)
  To: Edmundo Carmona Antoranz; +Cc: Derrick Stolee, dstolee, Git List, Jeff King

Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:

> 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-<.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-09-09 20:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-08 21:10 [PATCH v2] blame.c: replace instance of !oidcmp for oideq Edmundo Carmona Antoranz
2020-09-09 12:39 ` Derrick Stolee
2020-09-09 13:10   ` Edmundo Carmona Antoranz
2020-09-09 18:35   ` Junio C Hamano
2020-09-09 19:13     ` Edmundo Carmona Antoranz
2020-09-09 20:08       ` Junio C Hamano

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git