Re: [PATCH 1/5] bisect: read bisect paths with strbuf_getline()

On 02/15/2016 06:05 AM, Junio C Hamano wrote:

Moritz Neeb [off-list ref] writes:

quoted

quoted

You would also want to think about the necessity of strbuf_trim()
here.  Now strbuf_getline() would trim the trailing CR, would we
still need to call strbuf_trim() here?  The code will break if you
just remove the call, but on the other hand, you will realize that
the trimming done by calling it is excessive and unnecessary, once
you inspect the code and learn who writes the file being read here
and how.

I am not sure what you mean by excessive: How much can I assume that
the input is like expected? The files we are talking about are supposed
to be read and written by git only. But could be modified in theory with
an editor, right? Then things could break, right? This question maybe holds
true for the other patches as well, I still have to look into them.

These are all good questions you as a Git contributor to be asking
yourself, and I really like the fact that you are thinking aloud
here.

Thanks for this feedback I will try to continue this thinking aloud
whenever appropriate.

Sorry by the way for only answering every week - I currently only have
time to work on git during the weekend. What is (assuming familiarity
with the codebase) the expected "cooking"-time of a smaller patch? I'm
sure the answer is "depends", but maybe you can give away some experiences.