Re: [PATCH v5 0/2] bisect--helper: rewrite of check-term-format()

[Pranit Bauva <pranit.bauva@gmail.com>]

On Sun, May 8, 2016 at 11:53 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> On Sun, May 8, 2016 at 7:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Pranit Bauva <pranit.bauva@gmail.com> writes:
>>
>>> I completely missed your point and you want me to go the Eric Sunshine's way?
>>
>> I am neutral.
>>
>> When I read your response to Eric's "top down" suggestion, I didn't
>> quite get much more than "I started going this way, and I do not
>> want to change to the other direction.", which was what I had the
>> most trouble with.  If your justification for the approach to start
>> from building a tiny bottom layer that will need to be dismantled
>> soon and repeat that (which sounds somewhat wasteful) were more
>> convincing, I may have felt differently.
>
> Sorry if it seemed that "I have done quite some work and I don't want
> to scrape it off and redo everything". This isn't a case for me. I
> think of this as just a small part in the process of learning and my
> efforts would be completely wasted as I can still reuse the methods I

efforts would **not** be completely wasted

> wrote. This is still open for a "philosophical" discussion. I am
> assuming 1e1ea69fa4e is how Eric is suggesting.
>
> Why I think its better to go the subcommand way:
>  * I can test **C implementation** of the function to verify whether
> it was done in a proper way. By using a "top down" approach, I can
> make the test suite passing by running the code from git-bisect.sh not
> the re-written C code.
>  * I have made a very few minor mistakes in conversion of
> check_term_format() which just messed up my head (I actually spent 3
> days before Christian pointed out that '!' was missing). Imagine
> debugging the complete git-bisect.sh to find a very small error after
> a complete implementation.
>  * Using subcommands, I can easily verify whether I have done it the
> right way or not.
>  * It makes the review process very simple. The reviewers/testers can
> verify that my method is working correctly and well computers can
> detect errors better than humans.
>  * I can convert small functions which can be reviewed easily rather
> than dumping a big series.
>
> What I think is that the bottom up approach will make life easier for
> the me and for reviewer. Yes, it does make the code "unclean" for the
> time being and if it is between releases then all the more painful.
> Well, the latter part can be avoided by keeping it in next.
>
> Please point out if I am mistaken about anything.

There is also a third way in which I can go which is kind of merge of
both the ways:

 * Make two branches, one will contain the top down approach and
another will contain the subcommand approach.

 * I will write convert a function as a subcommand and test it locally
as well as on Travis-Cl by a PR on my git.git repo. The
reviewers/testers can consult the Travis-CI for output of the tests.
This will also reduce the need of locally compiling/testing.

 * I will then just write the function as it is in my "top down"
branch and thus send the patch to the mailing list. These can be kept
in next until it finishes.

 * Then will remove linking of git-bisect.sh and thus finalize.

This will be a bit more intensive not in a coding way but in
maintaining the repo. So I will have more experience of maintaining a
git repo. :)

Regards,
Pranit Bauva