From: Moritz Neeb Date: Sun, 21 Feb 2016 23:48:54 GMT Subject: Re: [PATCH 1/5] bisect: read bisect paths with strbuf_getline() Message-ID: <56CA4CE6.9070802@moritzneeb.de> In-Reply-To: On 02/15/2016 06:05 AM, Junio C Hamano wrote: > Moritz Neeb writes: > >>> 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.