Re: [RFC PATCH v2 0/2] bisect: add a single command for editing logs

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

 



Adam Dinwoodie <adam@xxxxxxxxxxxxx> writes:

> - It's possible to start a bisect session with a command like `git
>   bisect @ @~10`.  This will lead to the bisect log including the `@`
>   and `@~10` literally, and the interpretation of those values changes
>   depending on the current HEAD.  As a result, if you do a `git bisect
>   edit` after starting a bisect like that, but don't actually edit the
>   file, you'll nonetheless be in a different state.

This is a tangent, but for writing to the general public, please do
spell out HEAD, not the line noise synonym "@" that confuses readers.

>   I can see a few ways of coping with that:
>
>   1. Change the existing `git bisect start` behaviour to run arguments
>      through `git rev-parse` before recording them.  It appears `git
>      bisect good` et al. already do that, but it is a change in
>      behaviour that I guess could impact badly on other people using
>      `git bisect log`-based workflows.

The issue is not just HEAD but also for anything fruid, i.e. the
name of a branch, a search result ":/pattern", etc., and if we want
to allow restarting a previously failed bisect session from a
midpoint, we should be recording things in absolute terms as early
as possible.  I'd think it was an oversight the "log" thing did not
do so.

>   2. Do a full `git bisect reset` before replaying the log, so the
>      revisions will be parsed in the same way as they were originally.
>      I'd be slightly sad about that, as it seems an unnecessary
>      inefficiency, but it may well be the simplest approach.

It is not just inefficient, but would require there is no a local
change; I thought that the current system allows you to have a local
modification to a path that is not involved in the bisect session
and losing that property would be sad.

> - There aren't yet any tests or documentation changes; I wanted to get
>   commentary on the initial code changes before I spent time on those
>   parts.

There are some chicken-and-egg around this area.  For some changes,
without a doc update and test addition, it is harder to judge if a
reviewer can agree with the proposed change, as there is only a high
level description "we allow editing" and the lowest level changes to
the actual code, without anything in between that describes the
guiding principle and design decision that lead to the patch.

I'll need to see if the changes in the patch is clear/trivial enough
to see where you are trying to go to see if it is the case for this
patch, though, so read the above paragraph as a general guideline.





[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]

  Powered by Linux