On Sat, May 7, 2016 at 3:45 AM, Junio C Hamano <gits...@pobox.com> wrote: > Pranit Bauva <pranit.ba...@gmail.com> writes: > >> 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. > >> * 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
