On Sun, Feb 28, 2016 at 2:47 AM, Moritz Neeb <lists@xxxxxxxxxxxxx> wrote: > On 02/28/2016 07:56 AM, Eric Sunshine wrote: >> On Sun, Feb 28, 2016 at 12:13 AM, Moritz Neeb <lists@xxxxxxxxxxxxx> wrote: >>> The strbuf is now rtrimmed before splitting. This is still required >>> to remove potential CRs. In the next step this will then be done >>> implicitly by strbuf_readline(). Thus, this is a preparatory refactoring, >>> towards a trim-free codepath. >> >> I would actually swap patches 4 and 5 so that strbuf_getline() is done >> first (without removing any of the rtrim's) and string_list_split() >> second. That way, you don't have to add that extra rtrim in one patch >> and immediately remove it in the next. And, as a bonus, you can drop >> the above paragraph altogether. > > Yeah, I also was thinking about that, should've pointed that out. > I was just following your "guiding" in v2 [1], that's why I did it this way, > because I thought it is somehow expected to be a prepraratory change. Indeed, I meant to add a footnote acknowledging that and saying something along the lines of: "Despite suggesting this as a preparatory change in my v2 review, having now seen actually seen it, it makes more sense as a follow-on change." > Ok, when switching 4 and 5, I could call it something like "post cleanup/refactoring" > instead. > > [1] http://article.gmane.org/gmane.comp.version-control.git/286868 -- 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