Re: [PATCH] Warnings before rebasing -i published history

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Jun 7, 2012, 22:04 UTC
Message-ID: <vpqy5ny22z5.fsf@bauges.imag.fr>
In-Reply-To: <1339104053-7782-1-git-send-email-Lucien.Kong@ensimag.imag.fr>

Lucien Kong <Lucien.Kong@ensimag.imag.fr> writes:

> For now, the code only warns the user; he can't revert back to his
> original state. We agree with everyone that he should be given a
> way to abort the rebase.

He already has: empty the todo-list in the editor and you're done.

Show 9 quoted lines

> +warn_published () {
> +	cat "$1" | while read -r command sha1 message
> +	do
> +		test -n "$sha1" || break
> +		if test -n "$(git branch -r --contains "$sha1")"
> +		then
> +			printf "%s\n" "$(sed -e "/"$sha1"/ s|$| [Published]|" "$1")" >"$1"
> +		fi
> +	done

Aren't you reading and writing from the same file here? Sounds dangerous.

This appends [Published] to commits that are reachable from a remote-tracking branch, but I'm wondering if actually showing the remote branch name wouldn't be better, e.g.

pick <id> commit one # Commits above this line appear in origin/master pick <id> commit two pick <id> commit three # Commits above this line appear in origin/next pick <id> most recent commit

>  test -s "$todo" || echo noop >> "$todo"
>  test -n "$autosquash" && rearrange_squash "$todo"
> +warn_published "$todo"

That should be configurable.

Show 5 quoted lines

> @@ -869,6 +884,8 @@ cat >> "$todo" << EOF
>  #  f, fixup = like "squash", but discard this commit's log message
>  #  x, exec = run command (the rest of the line) using shell
>  #
> +# Warning: [Published] means that the commit has already been published

It's a pity to show the message unconditionally. If you go this way, skip the message if you didn't display [Published].

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/