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

On Sat, May 7, 2016 at 3:45 AM, Junio C Hamano [off-list ref] wrote:

Pranit Bauva [off-list ref] writes:

quoted

A very interesting fact to note:
I made a mistake by dropping of the first argument 'term' in the function
one_of() and compiled and the test suite passed fully (which was pointed
out by Christian). This shows that the test coverage is incomplete.
I am
currently writing tests to improve on that coverage. It will be included
separately.

Link to v4: http://thread.gmane.org/gmane.comp.version-control.git/293488

Changes wrt v4:
 * Stick with 'subcommand' in the commit message.
 * Rename enum as 'subcommand' to make it singular.

I've already said what needs to be said on this.

quoted

 * s/bug/BUG/g
 * Drop _() in the default of switch case
 * Use some suggestions for commit message no. 2 and also include why I am
   using subcommand.

I am not particularly impressed by the rationale, though.

As a starter project to show that you learned how to add a new mode
to bisect--helper, check-term-format may be a small enough function
that is easy to dip the toe into the codebase, so in that sense it
may be an appropriate first step, but otherwise it is not all that
interesting, especially when we _know_ that it will be discarded.

I do understand that check-term-format was a suggestion just for me to
"Get a taste" for the project.
I have now also converted write_terms() function. I have made a PR[1]
which needs a few things to be addressed.
Just a summary of what the PR does:
 * Converts the shell function write() terms to C
 * Adds a subcommand for write_terms()
 * Remove the subcommand for check_term_format()
 * Call the method check_term_format() from inside write_terms().

Do you want me to include both of the functions check_term_format()
and write_terms() in the same commit or a different commit.

OR,
I completely missed your point and you want me to go the Eric Sunshine's way?

[1]: https://github.com/pranitbauva1997/git/pull/3