Toon Claes <toon@xxxxxxxxx> writes:
> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>
[snip]
>> +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]
>> +
>> +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.
>> + )
>> +'
>> +
>> +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.
>> + )
>> +'
>> +
>> +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 :)
>> + )
>> +'
>> +
>> +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@xxxxxxxxxxxxxx
Attachment:
signature.asc
Description: PGP signature
