Re: [PATCH 5/6] status: do not depend on flaky reflog messages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ramkumar Ramachandra <artagnon@xxxxxxxxx> writes:

> Junio C Hamano wrote:
>> Two possibilities:
>>
>>  (1) Assume that the other thread will produce a more reasonable
>>      semantics when finished; perhaps the first line will go away
>>      entirely, or maybe it would say something like "# Rebasing;
>>      head at $commit".
>>
>>      Your topic does not _care_ what it would say, so you tweak the
>>      "status" test that is done during "rebase" so that they
>>      ignore the first lines; or
>
> You said you didn't want to regress to show senseless information,...

Go back and read it again.

    If you want to avoid regression, the codepath in wt-status.c
    should compensate for the change to "rebase" so that it checks
    $dotest/onto and show what is recorded in there.

> I agreed with that.  What is wrong with the patch I showed in the
> previous email?

Which previous?  My message you are responding to was a response to
your non-patch message, and tangling the In-reply-to backwards will
reach your original patch.

Puzzled....

> Smudging is a bad hack, and must only be used as a
> last resort: when an another topics updates status to say something
> sensible, it will have to unsmudge the tests.

Yes. I just got an impression, from your incessant arguing, that you
are resisting the "ignore the top" as "extra work that is not
interesting to _me_", while I was trying to see what is the best way
to go forward for the _project_.

Honestly, I didn't want to waste my time and was trying to come up
with a compromise, which would be a small regression to the tests
that we will need to fix with the other topic.  Because you already
said that you are not interested in that topic, I was anticipating
that the work to polish the topic would be a lot easier than
anything I would have to do with you in this topic, because others
are a lot easier to collaborate with than you are, and that is where
that suggestion comes from.

> diff --git a/wt-status.c b/wt-status.c
> index bf84a86..99c55e3 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1182,7 +1182,7 @@ void wt_status_print(struct wt_status *s)
>                                 if (!get_sha1("HEAD", sha1) &&
>                                     !hashcmp(sha1, state.detached_sha1))
>                                         on_what = _("HEAD detached at ");
> -                               else
> +                               else if (!state.rebase_in_progress)
>                                         on_what = _("HEAD detached from ");

Is this supposed to be on top of your original patch?

The primary reason I suggested to special case "rebase-in-progress"
in the "how about this" patch was because that way, you can have the
"have builtin/commit.c honor reflog-action" much earlier in the
series, because what this part of the code would say will not be
affected by what is recorded in the reflog.  If the above, together
with the original patch, makes the code match the expectation of the
"rebase stops in the middle and then status runs" tests, that's also
fine.  The other topic needs to redo it in either case anyway.

> Unless we change the first line drastically to say: "rebase in
> progress: rebasing onto $ONTO" (or something), I don't think this
> makes sense.  And if we were to do that, why not do it properly like
> "rebase ($N/$M): onto $ONTO, upstream $UPSTREAM, branch $BRANCH"?
> Other people on a different thread are already handling that,...

That is the thread I was referring to.


>
> So, you have three simple choices now:
>
> 1. Accept the simple patch I proposed above.
> 2. Propose an alternative patch quickly.  *Patch*.  No more English.
> 3. Reject all patches, and leave me no choice but to smudge.
>
> Which one is it going to be?

None of the above.  After going back and forth this long, I won't
want to pick an incremental from the middle of discussion, so (1) is
out.  This is not my itch and I am only helping you in a way that
the result will not hurt the project, so (2) is not it.  But
assuming that "checkout that is done as an implementation detail in
rebase is _not_ checkout from end user's point of view" is where we
want to go, discarding this series is not something we want to see,
either.

A rerolled series that does:

 - Tweak wt-status.c to take information not from reflog, when
   detached during rebase (this may need to tweak existing tests,
   as different "rebase" modes may use 'checkout' liberally in
   different ways);

 - Teach builtin/commit.c to pay attention to reflog-action; thanks
   to the previous step, this will not have to change the tests;

 - Update the reflog message rebase uses, but again this will not
   affect wt-status.c at all.

would be one way to go.



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]