On 04/11/2018 04:23 PM, Jim Jagielski wrote: > +1 on keeping the patch as is and not breaking it out as a "refactor" plus > a new feature.
Definitely for this case. OTOH if practically possible I like to see stuff in separate commits to ease reviewing and later error diagnosis. But as said this is not always practical and is always a trade-off. > > We have seen WAY too many cases where a refactor has caused > issues. I'm not saying we shouldn't fix things, but major refactors > should, IMO, have a real-world need, and tangible improvement, > other than "I want it to work this way." Agreed, but real-world needs IMHO include performance improvements and better readability / maintainability of code and as such are not not necessarily functional changes. But I am well aware that what is more readable or maintainable is easily a subject of personal taste :-) Regards Rüdiger
