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

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

 



On Sat, May 7, 2016 at 3:45 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Pranit Bauva <pranit.bauva@xxxxxxxxx> 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 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]