[dmd-internals] Refactorings of dmd

[Brad Roberts via dmd-internals]

Thanks for speaking up Steven. I've been wanting to write something 
similar, but with a greater focus on the stated but not demonstrated tie 
between refactoring and regressions.

There were a number of statements asserted as facts without any 
evidence.  I doubt there's even any correlation much less causation. 
Additionally, it was stated as a fact that there was a sudden increase 
in regressions when I don't believe that's true either.  There has been 
a steady accumulation of them over time.  The current count _is_ way 
larger than I'd like to see, but that's not sudden.  In fact, it's a 
very predictable recurring pattern in the D development cycle. 
Regressions are typically focused on primarily just before a release is 
to be made.

However, I wanted to do the digging to refute these points with data and 
just haven't had the energy.  However, leaving this unchallenged is even 
worse than well challenged, so, I'll leave this with just as few facts 
as Walter's original assertion.  My apologies.

My 2 cents,
Brad

On 5/25/2015 11:28 AM, Steven Schveighoffer via dmd-internals wrote:
> I feel like I need to respond to this. For the most part, I think the “bouncing the rubble” points are spot on. Reorganizing files can have benefits, and generally github is quite good at recognizing the move. But other than that, changing function names/capitalization/etc is kind of pointless and creates more rift/cruft.
>
> But the logical fallacy of regressions being tied to refactorings (correlation does not imply causation) bothers me. Regressions aren’t exclusively caused by refactorings, and I think I’ve seen quite a few caused by moving non-template functions to templates (this causes lots of headaches because testing a template is a much different animal). Sometimes progress reveals mistakes that you learn from, or identifies more issues to be fixed. The on-purpose breaking needs a high bar, and you have discretion there. I have seen a few times recently where you rejected very popular on-purpose breakages that seem for some to make progress, and that’s fine, I defer to your wisdom there, even if I don’t agree. But please don’t consider this to be carelessness or explicit sabotage. Everyone is trying to improve things.
>
> And finally, the lamenting about not having everyone working on what you want to do is somewhat insulting. Not everyone has the same goals or dreams, and many of these pull requests come from outside our developer circle. When someone feels passionately about a problem, and submits a fix, I think it’s foolish and damaging to reject that or ignore it simply because it’s not part of the Walter Bright plan. We all have our selfish wants and needs, and that is not a bad thing. We are told day in and day out, if you want something changed in D, please submit a PR. I understand you’re busy, it’s a frequent problem of mine too.
>
> There are many things that I think are critical to the success of D that nobody works on. But I don’t complain that nobody listens to me. And to be fair, there have been quite a few pulls generated by others to further your goal of rangifying functions that others have made.
>
> Please keep in mind how open source communities work together, this isn’t the same as a for-pay organization. We all agree with your vision for the most part, but not everyone needs to exclusively work on your priorities for D to succeed.
>
> Perhaps your rant was directed specifically at certain people/changes. I can’t tell from your post. But I hope you can understand how discouraging this sounds to others, no matter what part of the system they work on. I am really in agreement that we want to prevent needless rubble movement. It’s just the lack of your agenda fulfillment that gets me.
>
> Finally, please note that these statements are from ME I don’t represent anyone else. And I’m not trying to cause distress here. I’m merely stating how this sounds to me. Let’s have a beer in Utah and get our goals all aligned :)
>
> -Steve
>
>> On May 22, 2015, at 6:07 PM, Walter Bright via dmd-internals <dmd-internals at puremagic.com> wrote:
>>
>>
>>
>> On 5/22/2015 10:16 AM, Daniel Murphy wrote:
>>> Could you please identify some refactorings that you feel are useless?
>>>   I've been the author of many large refactorings needed for DDMD, and
>>> I suspect you making yourself the bottleneck for refactorings is going
>>> to be a huge pain.  If you feel contributors have been merging pull
>>> requests without adequate review please let us know.
>>>
>>
>> I have pulled the ones you needed for DDMD, and I pulled them because it was necessary in order to make DDMD work.
>>
>> Of course it's a judgement call, but refactorings that are not productive share one or more of these characteristics:
>>
>> 1. potatoe potahto name changes
>> 2. indentation/whitespace/reformatting
>> 3. the number of lines of code increases
>> 4. re-ordering functions in a file
>> 5. moving code from one file to another
>> 6. large diffs with no seeming point to them
>> 7. large diffs that are not reviewable because github diff is unhelpful with figuring out what actually changed
>> 8. make dmd slower
>>
>> Essentially, refactorings that "bounce the rubble" around are going to be viewed negatively.
>>
>> Refactorings that would be viewed with favor:
>>
>> 1. identifying significant common code sequences out and consolidating into reusable functions
>> 2. better encapsulation
>> 3. more logical flow of control
>> 4. reduction in cyclomatic complexity
>> 5. move towards making functions pure
>>
>> For example, I recently did a small refactor that replaced argc/argv manual memory management with the Strings type. It removed a bunch of code, and the rest flowed a lot better.
>>
>> I've pulled a lot of refactorings. My concern with this is the large increase in regressions. Those refactorings should be reducing regressions - but it seems that at best they are not helping, and at worst are making things worse. I'm also concerned that people are not working on the real problems with DMD, and instead are doing refactorings. We have a lot of open regressions, and no proposed fixes for them. But we've got refactorings regularly appearing. I know that refactorings are more fun than regression fixes. But we've got to get the bread and butter work done.
>>
>> And lastly, I am swamped with work. I'm the only one working on range fixes for Phobos - fixes that I have been advocating for a couple years now, and nobody has done anything to move this forward. So I am doing it. This range-ification is, quite frankly, absolutely critical to D's future success. I must get it done. But I get bogged down in interminable reviews of refactorings with hundreds of lines of rubble bouncing.
>> _______________________________________________
>> dmd-internals mailing list
>> dmd-internals at puremagic.com
>> http://lists.puremagic.com/mailman/listinfo/dmd-internals
>
>
> _______________________________________________
> dmd-internals mailing list
> dmd-internals at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals
>