threads / patch / 44967

patchfixup! worktree move: new command

Subject: [PATCH] fixup! worktree move: new command

## tl;dr

4 messages between Jan 25, 2017 and Jan 30, 2017. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Johannes Schindelin· Jan 25, 2017, 13:04 UTC · lore

This is required for the test to pass on Windows, where $TRASH_DIRECTORY is a POSIX path, while Git works with Windows paths instead. Using `$(pwd)` is the common workaround: it reports a Windows path (while `$PWD` would report the POSIX equivalent which, however, would only be understood by shell and Perl scripts).

Duy, if you re-roll the `worktree-move` patch series, would you terribly mind squashing this in?

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Based-On: pu at https://github.com/dscho/git
Fetch-Base-Via: git fetch https://github.com/dscho/git pu
Published-As: https://github.com/dscho/git/releases/tag/nd/worktree-move-fixup-v1
Fetch-It-Via: git fetch https://github.com/dscho/git nd/worktree-move-fixup-v1

 t/t2028-worktree-move.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Show changes to t/t2028-worktree-move.sh +4 −3

diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 084acc6c6d..b3105eaaed 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -71,13 +71,14 @@ test_expect_success 'move locked worktree' '
 '
 
 test_expect_success 'move worktree' '
+	toplevel="$(pwd)" &&
 	git worktree move source destination &&
 	test_path_is_missing source &&
 	git worktree list --porcelain | grep "^worktree" >actual &&
 	cat <<-EOF >expected &&
-	worktree $TRASH_DIRECTORY
-	worktree $TRASH_DIRECTORY/destination
-	worktree $TRASH_DIRECTORY/elsewhere
+	worktree $toplevel
+	worktree $toplevel/destination
+	worktree $toplevel/elsewhere
 	EOF
 	test_cmp expected actual &&
 	git -C destination log --format=%s >actual2 &&

base-commit: 8ef3497bf10296c74058e87d9c5b93372cad5137

-- 
2.11.0.windows.3.498.g86d49cc

Johannes Schindelin· Jan 27, 2017, 10:53 UTC · re: Johannes Schindelin · lore

Re: [PATCH] fixup! worktree move: new command

Hi Junio,

On Wed, 25 Jan 2017, Johannes Schindelin wrote:

Show 11 quoted lines

> This is required for the test to pass on Windows, where $TRASH_DIRECTORY
> is a POSIX path, while Git works with Windows paths instead. Using
> `$(pwd)` is the common workaround: it reports a Windows path (while `$PWD`
> would report the POSIX equivalent which, however, would only be understood
> by shell and Perl scripts).
> 
> Duy, if you re-roll the `worktree-move` patch series, would you terribly
> mind squashing this in?
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---

Given that you add all kinds of SQUASH patches in `pu` that have not even always been discussed on the mailing list, I would like to kindly ask you to please add this patch to the nd/worktree-move branch for the time being (i.e. until Duy responds), so that I am not continuously pestered by failing Continuous Integration builds about a problem for which I have provided a fix already.

Thanks, Johannes

Junio C Hamano· Jan 27, 2017, 17:51 UTC · re: Johannes Schindelin · lore

Re: [PATCH] fixup! worktree move: new command

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> always been discussed on the mailing list, I would like to kindly ask you
> to please add this patch to the nd/worktree-move branch for the time being
> (i.e. until Duy responds),

The tip of 'pu' (or anything beyond the tip of 'jch') is not always expected to pass test or even build, and unless I know the OP is leaky or usually slow, a "fixup!" that directly addresses the OP tends to be left for the OP to pick up [*1*] to avoid "ah, you sent a reroll but I already had one squashed locally", which is confusing to both myself and OP (and adds burden to me).

It seems that OP is slower to respond than his usual, so I'll add it to the tip of the topic so that it won't be lost.

[Footnote]

*1* It still is used on my end to leave a mental note to myself that
    the topic is expected/waiting to be rerolled, but that is not
    something you can read in "git log --first-parent master..pu".

Johannes Schindelin· Jan 30, 2017, 12:24 UTC · re: Junio C Hamano · lore

Re: [PATCH] fixup! worktree move: new command

Hi Junio,

On Fri, 27 Jan 2017, Junio C Hamano wrote:

> The tip of 'pu' (or anything beyond the tip of 'jch') is not always
> expected to pass test or even build, [...]

This makes `pu` a lot less useful than it could be.

And we could easily improve the situation simply by changing the rule ever so slightly: when a build, or a test, fails in `pu` and there exists a fix, this fix should go into `pu` ASAP.

As you point out later in your mail, the fixup! or SQUASH! commit is a very convenient reminder that a particular branch is still "under construction". That is, changing the rule as I proposed above will not only help the Continuous Integration [*1*] to avoid reporting duplicates, it will also help us improve the project faster.

Ciao, Johannes

Footnote *1*: It appears that there may be the misconception floating around that Continuous Integration is designed to annoy developers by pointing out unportable or unbuildable code. Once you realize, though, that it detects and reports code that is below our existing code's quality, no doubt you will agree that it is a convenient tool to relieve reviewers from tedious work that can be done by a machine as well.