CJCombrink commented on PR #2943: URL: https://github.com/apache/thrift/pull/2943#issuecomment-1986930547
@Jens-G Sorry I did not do the research before making the PR, but the linked ticket confirms my suspicion 100%: A bad practice was followed and instead of fixing the actual issue, by not using `std::endl`, the PR decided to entertain a bad practice with the motivation of "minimizing code churn" by changing expected behaviour with hidden unexpected behaviour (PS I was fully aware of that constant before making this PR). 9 Years later I am hoping that at least we can get to a point where sanity is restored and bad practices are not encouraged/entertained in such important code bases. PS: I come from a background where juniors used `std::endl` on embedded devices that flushed over serial. On confrontation the response was "_we were taught that way_". We should be better in teaching and educating on these matters, not hide it with `consts` that change documented behaviour and make bad practices acceptable. I am saying this again, I realise I am passionate about the wrong things thus I will not take offence if this/these PR(s) is/are declined, but I might keep stating the point :P @emmenlau > advantages of recognizing `<< endl <<` as a clear sign of newline Depends on perspective: I see it as: "why do you need flushing" and my editor (VS Code with no custom modification) highlights `\n` differently in string literals, thus I can argue it stands out even more and communicates the intend clearly (no hidden oh but we manually changed behaviour). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
