On Fri, Mar 25, 2016 at 2:45 PM, Matthieu Moy
<matthieu....@grenoble-inp.fr> wrote:
> Christian Couder <christian.cou...@gmail.com> writes:
>
>> On Thu, Mar 24, 2016 at 12:27 AM, Pranit Bauva <pranit.ba...@gmail.com>
>> wrote:
>>
>>> Unification of bisect.c and bisect--helper.c
>>>
>>> This will unify the algorithmic and non-algorithmic parts of bisect
>>> bringing them under one heading to make the code clean.
>>
>> I am not sure this is needed and a good idea. Maybe you will rename
>> "builtin/bisect--helper.c" to "builtin/bisect.c" and remove
>> git-bisect.sh at the same time to complete the shell to C move. But
>> the actual bisect.{c,h} might be useful as they are for other
>> purposes.
>
> Yes. My view on this is that builtin/*.c should be just user-interface,
> and actual stuff should be outside builtin, ideally in a well-designed
> and reusable library (typically re-usable by libgit2 or others to
> provide another UI for the same feature). Not all commands work this
> way, but I think this is a good direction to take.
Okay. I didn't know about this. Thanks for completing Christian's point.
>> When you have sent one patch series, even a small one, then your main
>> goal should be to have this patch series merged.
>
> I'd add: to get a patch series merged, two things take time:
>
> 1) latency: let time to other people to read and comment on your code.
>
> 2) extra-work required by reviewers.
>
> You want to send series early because of 1) (then you can work on the
> next series while waiting for reviews on the current one), and you need
> to prioritize 2) over working on the next series to minimize in-flight
> topics.
I had planned to work this way. I will include this in the proposal.
Though it creates some confusion for me and I tend to mix some things
up but I will maintain a hard copy to jot down the discussions and my
thoughts.
--
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
