Thread (20 messages) 20 messages, 5 authors, 2025-03-19

Re: [PATCH v3 2/2] reflog: implement subcommand to drop reflogs

From: Karthik Nayak <hidden>
Date: 2025-03-19 09:16:11

Toon Claes [off-list ref] writes:
Karthik Nayak [off-list ref] writes:
[snip]
quoted
+static int cmd_reflog_drop(int argc, const char **argv, const char *prefix,
+			   struct repository *repo)
+{
+	int ret = 0, do_all = 0, single_worktree = 0;
+	const struct option options[] = {
+		OPT_BOOL(0, "all", &do_all, N_("drop the reflogs of all references")),
+		OPT_BOOL(0, "single-worktree", &single_worktree,
+			 N_("drop reflogs from the current worktree only")),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, options, reflog_drop_usage, 0);
+
+	if (argc && do_all)
+		usage(_("references specified along with --all"));
What is the intended behavior when both `--all` and `<refs>` are
omitted? It seems nothing happens at the moment. And no error nor
warning is printed, that feels a bit odd to me.

Now, when you do `git reflog expire --expire=all` it also seems to be
doing nothing at all. I also think this is weird. And I don't see any
test coverage for `git reflog expire` without `--all`.

But what is the expected behavior when you omit `--all` and `<refs>`?
Should it give an error or warning? Should it use HEAD, just like `git
reflog show` does?
As discussed in the other thread [1], ideally this should be raised as
an error. I'm leaving it for now.

[snip]
quoted
+
+test_expect_success 'reflog drop --all' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A &&
+		test_commit_bulk --ref=refs/heads/branch 1 &&
+		git reflog exists refs/heads/main &&
+		git reflog exists refs/heads/branch &&
+		git reflog drop --all &&
+		test_must_fail git reflog exists refs/heads/main &&
+		test_must_fail git reflog exists refs/heads/branch
Should we test output of `git reflog list`?
I don't see why, we're concerned with individual reflogs and 'exists'
help check against those individual reflogs.
quoted
+	)
+'
+
+test_expect_success 'reflog drop --all multiple worktrees' '
+	test_when_finished "rm -rf repo" &&
+	test_when_finished "rm -rf wt" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A &&
+		git worktree add ../wt &&
+		test_commit_bulk -C ../wt --ref=refs/heads/branch 1 &&
+		git reflog exists refs/heads/main &&
+		git reflog exists refs/heads/branch &&
+		git reflog drop --all &&
+		test_must_fail git reflog exists refs/heads/main &&
+		test_must_fail git reflog exists refs/heads/branch
Shall we test HEAD in both worktrees does not exists?
I think it would be a good addition, but I'm not sure if its worthy of a
re-roll.
quoted
+	)
+'
+
+test_expect_success 'reflog drop --all --single-worktree' '
+	test_when_finished "rm -rf repo" &&
+	test_when_finished "rm -rf wt" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A &&
+		git worktree add ../wt &&
+		test_commit -C ../wt foobar &&
+		git reflog exists refs/heads/main &&
+		git reflog exists refs/heads/wt &&
+		test-tool ref-store worktree:wt reflog-exists HEAD &&
+		git reflog drop --all --single-worktree &&
+		test_must_fail git reflog exists refs/heads/main &&
+		test_must_fail git reflog exists refs/heads/wt &&
+		test_must_fail test-tool ref-store worktree:main reflog-exists HEAD &&
+		test-tool ref-store worktree:wt reflog-exists HEAD
Naive question: why is `test-tool ref-store` used and not
`git -C ../wt reflog exist`?
That should work too :)
quoted
+	)
+'
+
+test_expect_success 'reflog drop --all with reference' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A &&
+		test_must_fail git reflog drop --all refs/heads/main 2>stderr &&
+		test_grep "usage: references specified along with --all" stderr
+	)
+'
+
 test_done

--
2.48.1
[1]: CAOLa=ZSj11TSTs6CywSX1Q9AAfW28zssS2yrGf8PmBOgd06Etg@mail.gmail.com

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help