git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Aaron Lipman <alipman88@gmail.com>
To: git@vger.kernel.org
Cc: Aaron Lipman <alipman88@gmail.com>
Subject: [PATCH v4 0/5] Introduce --first-parent flag for git bisect
Date: Tue,  4 Aug 2020 18:01:08 -0400	[thread overview]
Message-ID: <20200804220113.5909-1-alipman88@gmail.com> (raw)
In-Reply-To: <20200801175840.1877-1-alipman88@gmail.com>

OK, here's take 4! Responding to Junio's feedback, first:

> That function [cmd_bisect__helper()] is supposed to be a thin shim
> layer whose only reason of its existence is to serve as an interface
> with the scripted version of "git bisect".  When everything is
> migrated from the shell script, cmd_bisect__helper() should disappear.
> ...
> This is going backwards, as far as I can tell.  If anything, I'd
> rather see cmd_bisect__helper() get fixed so that it does not parse
> "--first-parent" (and similarly, "--no-checkout" that you imitated)
> into first_parent_only (and no_checkout) variables and passed as
> parameters to bisect_start().

Thanks for the explanation, Junio. (And for bearing with me while I
gain familiarity with git's codebase.) Now that I've taken some time
to examine how git-bisect.sh and cmd_bisect__helper() fit together,
the correct approach is much more clear.

I've added a commit (3/5 in this iteration) that updates
cmd_bisect__helper() to defer parsing the --no-checkout flag.

As no_checkout was also passed to bisect_next_all() [via implicitly
checking for the existence of .git/BISECT_HEAD when calling
bisect_next() in git-bisect.sh], I've removed that parameter and
instead check for .git/BISECT_HEAD in bisect_next_all() to determine
whether no-checkout mode is on.

This means no-checkout mode can no longer be enabled by "git
bisect--helper --next-all --no-checkout" - it can now only be turned
on when running "git bisect start". But this doesn't change git
bisect's public API, so this should be OK.

Other than the changes suggested by Martin (summarized below), the
only new change is removing the read_first_parent_option() wrapper
function in favor of file_exists() from dir.h.

---

Martin, thanks for your suggestions - I've moved the commit updating
"git bisect run" tests to 1/5, updated the commit message, and
included the changes you provided. (I held off on additional
indentation corrections, as it kinda felt like unnecessary code
churn/outside the scope of abstracting platform-specific details.)

> Also, does "separate between <one thing>" make grammatical sense?
> Should it be "separate between <two things>" or "separate [it] from
> <one thing>"? Dunno..

I think "separate [it] from <one thing>" is correct, good catch!

> (Signed-off-by: Martin Ågren <martin.agren@gmail.com>, FWIW.)

I'm still getting used to the conventions - should I add your name as
a signed-off-by tag, a thanks-to tag, or both?

Aaron Lipman (5):
  t6030: modernize "git bisect run" tests
  rev-list: allow bisect and first-parent flags
  cmd_bisect__helper: defer parsing no-checkout flag
  bisect: introduce first-parent flag
  bisect: combine args passed to find_bisection()

 Documentation/git-bisect.txt       |  13 +++-
 Documentation/rev-list-options.txt |   7 +-
 bisect.c                           |  79 +++++++++++++---------
 bisect.h                           |   9 +--
 builtin/bisect--helper.c           |  23 ++++---
 builtin/rev-list.c                 |   9 ++-
 git-bisect.sh                      |   2 +-
 revision.c                         |   3 -
 t/t6000-rev-list-misc.sh           |   4 +-
 t/t6002-rev-list-bisect.sh         |  45 +++++++++++++
 t/t6030-bisect-porcelain.sh        | 104 ++++++++++++++++-------------
 11 files changed, 195 insertions(+), 103 deletions(-)

-- 
2.24.3 (Apple Git-128)


  parent reply	other threads:[~2020-08-04 22:01 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-28 15:44 [PATCH 0/3] Introduce --first-parent flag for git bisect Aaron Lipman via GitGitGadget
2020-07-28 15:44 ` [PATCH 1/3] rev-list: allow bisect and first-parent flags Aaron Lipman via GitGitGadget
2020-07-28 20:23   ` Junio C Hamano
2020-07-28 21:53     ` Junio C Hamano
2020-07-28 15:44 ` [PATCH 2/3] bisect: introduce first-parent flag Aaron Lipman via GitGitGadget
2020-07-28 15:44 ` [PATCH 3/3] bisect: combine args passed to find_bisection() Aaron Lipman via GitGitGadget
2020-07-30  0:27 ` [PATCH v2 0/3] Introduce --first-parent flag for git bisect Aaron Lipman
2020-07-30  0:27   ` [PATCH v2 1/3] rev-list: allow bisect and first-parent flags Aaron Lipman
2020-07-30  0:27   ` [PATCH v2 2/3] bisect: introduce first-parent flag Aaron Lipman
2020-07-30  4:17     ` Junio C Hamano
2020-07-30  0:27   ` [PATCH v2 3/3] bisect: combine args passed to find_bisection() Aaron Lipman
2020-07-30  4:32     ` Junio C Hamano
2020-07-30  0:48   ` [PATCH v2 0/3] Introduce --first-parent flag for git bisect Junio C Hamano
2020-08-01 17:58   ` [PATCH v3 0/4] " Aaron Lipman
2020-08-01 17:58     ` [PATCH v3 1/4] rev-list: allow bisect and first-parent flags Aaron Lipman
2020-08-01 17:58     ` [PATCH v3 2/4] bisect: introduce first-parent flag Aaron Lipman
2020-08-01 17:58     ` [PATCH v3 3/4] bisect: combine args passed to find_bisection() Aaron Lipman
2020-08-01 19:30       ` Martin Ågren
2020-08-01 17:58     ` [PATCH v3 4/4] bisect: consistent style for git bisect run tests Aaron Lipman
2020-08-01 19:27       ` Martin Ågren
2020-08-01 19:06     ` [PATCH v3 0/4] Introduce --first-parent flag for git bisect Junio C Hamano
2020-08-04 22:01     ` Aaron Lipman [this message]
2020-08-04 22:01       ` [PATCH v4 1/5] t6030: modernize "git bisect run" tests Aaron Lipman
2020-08-05  6:11         ` Christian Couder
2020-08-04 22:01       ` [PATCH v4 2/5] rev-list: allow bisect and first-parent flags Aaron Lipman
2020-08-05  0:38         ` Eric Sunshine
2020-08-04 22:01       ` [PATCH v4 3/5] cmd_bisect__helper: defer parsing no-checkout flag Aaron Lipman
2020-08-04 22:01       ` [PATCH v4 4/5] bisect: introduce first-parent flag Aaron Lipman
2020-08-04 22:01       ` [PATCH v4 5/5] bisect: combine args passed to find_bisection() Aaron Lipman
2020-08-04 22:19       ` [PATCH v4 0/5] Introduce --first-parent flag for git bisect Junio C Hamano
2020-08-05  5:55       ` Christian Couder
2020-08-07 21:05         ` Junio C Hamano
2020-08-07 21:17           ` Eric Sunshine
2020-08-07 22:07             ` Junio C Hamano
2020-08-07 22:20               ` Eric Sunshine
2020-08-05  6:18       ` Martin Ågren
2020-08-07 21:58       ` [PATCH v5 " Aaron Lipman
2020-08-07 21:58         ` [PATCH v5 1/5] t6030: modernize "git bisect run" tests Aaron Lipman
2020-08-07 21:58         ` [PATCH v5 2/5] rev-list: allow bisect and first-parent flags Aaron Lipman
2020-08-07 21:58         ` [PATCH v5 3/5] cmd_bisect__helper: defer parsing no-checkout flag Aaron Lipman
2020-08-07 21:58         ` [PATCH v5 4/5] bisect: introduce first-parent flag Aaron Lipman
2020-08-07 21:58         ` [PATCH v5 5/5] bisect: combine args passed to find_bisection() Aaron Lipman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200804220113.5909-1-alipman88@gmail.com \
    --to=alipman88@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).