All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Zach FettersMoore via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Zach FettersMoore <zach.fetters@apollographql.com>
Subject: Re: [PATCH] subtree: fix split processing with multiple subtrees present
Date: Mon, 18 Sep 2023 16:31:34 -0700	[thread overview]
Message-ID: <xmqq34zbjbxl.fsf@gitster.g> (raw)
In-Reply-To: <pull.1587.git.1695067516192.gitgitgadget@gmail.com> (Zach FettersMoore via GitGitGadget's message of "Mon, 18 Sep 2023 20:05:16 +0000")

"Zach FettersMoore via GitGitGadget" <gitgitgadget@gmail.com>
writes:

>  contrib/subtree/git-subtree.sh | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index e0c5d3b0de6..e9250dfb019 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -778,12 +778,29 @@ ensure_valid_ref_format () {
>  		die "fatal: '$1' does not look like a ref"
>  }
>  
> +# Usage: check if a commit from another subtree should be ignored from processing for splits
> +should_ignore_subtree_commit () {
> +  if [ "$(git log -1 --grep="git-subtree-dir:" $1)" ]
> +  then
> +    if [[ -z "$(git log -1 --grep="git-subtree-mainline:" $1)" && -z "$(git log -1 --grep="git-subtree-dir: $dir$" $1)" ]]
> +    then
> +      return 0
> +    fi
> +  fi
> +  return 1
> +}
>
>  # Usage: process_split_commit REV PARENTS
>  process_split_commit () {
>  	assert test $# = 2
>  	local rev="$1"
>  	local parents="$2"
>  
> +    if should_ignore_subtree_commit $rev
> +    then
> +	    return
> +    fi
> +

Please do not violate Documentation/CodingGuidelines for our shell
scripted Porcelain, even if it is a script in contrib/ and also
please avoid bash-isms.

Also doesn't "subtree" have its own test?  If this change is a fix
for some problem(s), can we have a test or two that demonstrate how
the current code without the patch is broken?

Thanks.


  reply	other threads:[~2023-09-18 23:31 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-18 20:05 [PATCH] subtree: fix split processing with multiple subtrees present Zach FettersMoore via GitGitGadget
2023-09-18 23:31 ` Junio C Hamano [this message]
2023-09-19  1:04 ` Junio C Hamano
2023-10-26 19:59   ` Zach FettersMoore
2023-09-22 16:25 ` [PATCH v2 0/2] " Zach FettersMoore via GitGitGadget
2023-09-22 16:25   ` [PATCH v2 1/2] " Zach FettersMoore via GitGitGadget
2023-09-22 16:25   ` [PATCH v2 2/2] subtree: changing location of commit ignore processing Zach FettersMoore via GitGitGadget
2023-09-29 20:32   ` [PATCH v3 0/3] subtree: fix split processing with multiple subtrees present Zach FettersMoore via GitGitGadget
2023-09-29 20:32     ` [PATCH v3 1/3] " Zach FettersMoore via GitGitGadget
2023-09-29 20:32     ` [PATCH v3 2/3] subtree: changing location of commit ignore processing Zach FettersMoore via GitGitGadget
2023-09-29 20:33     ` [PATCH v3 3/3] subtree: adding test to validate fix Zach FettersMoore via GitGitGadget
2023-10-17 20:02       ` Zach FettersMoore
2023-10-26 19:17     ` [PATCH v4] subtree: fix split processing with multiple subtrees present Zach FettersMoore via GitGitGadget
2023-11-18 11:28       ` Christian Couder
2023-11-28 21:04         ` Zach FettersMoore
2023-11-28 21:17       ` [PATCH v5] " Zach FettersMoore via GitGitGadget
2023-11-30 20:33         ` Christian Couder
2023-11-30 21:01           ` Zach FettersMoore
2023-12-01 14:54         ` [PATCH v6] " Zach FettersMoore via GitGitGadget
2023-12-04 11:08           ` Christian Couder
2023-12-11 15:39             ` Zach FettersMoore
2023-12-12 16:06               ` Christian Couder
2023-12-12 22:28                 ` Junio C Hamano
2023-12-13 15:20                   ` Zach FettersMoore
2024-01-03 16:33                     ` Christian Couder
2023-12-20 15:25                 ` Christian Couder
2024-01-25 10:09                   ` Christian Couder
2024-01-25 16:38                     ` Junio C Hamano
2024-01-25 18:52                       ` Christian Couder
2024-01-25 18:56                         ` Junio C Hamano
2025-08-21  3:13           ` subtree: [v2.44 regression] split may produce different history Colin Stagner

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

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

  git send-email \
    --in-reply-to=xmqq34zbjbxl.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=zach.fetters@apollographql.com \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.