Jump to page: 1 2 3
Thread overview
[dmd-internals] Refactorings of dmd
May 22, 2015
Walter Bright
May 22, 2015
Daniel Murphy
May 22, 2015
Walter Bright
May 22, 2015
Daniel Murphy
May 23, 2015
Walter Bright
May 25, 2015
Brad Roberts
May 25, 2015
Jonathan M Davis
May 25, 2015
Walter Bright
May 26, 2015
Leandro Lucarella
May 26, 2015
Walter Bright
Jun 20, 2015
Martin Nowak
Jun 20, 2015
Iain Buclaw
Jun 20, 2015
Martin Nowak
Jul 09, 2015
Martin Nowak
Jul 25, 2015
Martin Nowak
May 22, 2015
We have a big problem with dmd:

https://issues.dlang.org/buglist.cgi?bug_severity=regression&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&list_id=201147&query_format=advanced

A large and rapidly growing regression list. Something is going seriously wrong.

We've also had a number of very large refactorings of dmd code. I do not see how this has improved anything. In fact, they make things worse by making it hard to trace through what went wrong, as the code is all shuffled around.

(Vladimir has done a great job in isolating which pull requests caused which regression.)

Until this gets under control:

1. There will be no more refactorings unless approved by myself.

2. I want to focus on cutting down that regression list, not adding ever more features.


_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals
May 22, 2015
> On May 22, 2015, at 3:18 AM, Walter Bright via dmd-internals <dmd-internals@puremagic.com> wrote:
> 
> We have a big problem with dmd:
> 
> https://issues.dlang.org/buglist.cgi?bug_severity=regression&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&list_id=201147&query_format=advanced
> 
> A large and rapidly growing regression list. Something is going seriously wrong.
> 

FYI, I found at least one regression (https://issues.dlang.org/show_bug.cgi?id=14530 <https://issues.dlang.org/show_bug.cgi?id=14530>) that was not properly closed by the github hook after the PR was pulled.

Make sure all these are still actually open. And please ensure when your PR gets pulled that the bug gets closed, sometimes this doesn’t happen automatically.

-Steve



May 23, 2015
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.

On Fri, May 22, 2015 at 5:18 PM, Walter Bright via dmd-internals <dmd-internals@puremagic.com> wrote:
> We have a big problem with dmd:
>
> https://issues.dlang.org/buglist.cgi?bug_severity=regression&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&list_id=201147&query_format=advanced
>
> A large and rapidly growing regression list. Something is going seriously wrong.
>
> We've also had a number of very large refactorings of dmd code. I do not see how this has improved anything. In fact, they make things worse by making it hard to trace through what went wrong, as the code is all shuffled around.
>
> (Vladimir has done a great job in isolating which pull requests caused which
> regression.)
>
> Until this gets under control:
>
> 1. There will be no more refactorings unless approved by myself.
>
> 2. I want to focus on cutting down that regression list, not adding ever more features.
>
>
> _______________________________________________
> dmd-internals mailing list
> dmd-internals@puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals
May 22, 2015

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@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals
May 23, 2015
Thanks Walter.  I think we're on the same page here.

On Sat, May 23, 2015 at 8:07 AM, Walter Bright <walter@digitalmars.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@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals
May 22, 2015
On 5/22/15 3:07 PM, Walter Bright via dmd-internals wrote:
> 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.

I quoted this all because I so much agree with it. Brilliantly put. -- Andrei

_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals
May 22, 2015

On 5/22/2015 4:16 PM, Daniel Murphy wrote:
> Thanks Walter.  I think we're on the same page here.
>

Thanks for your support with this.
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals
May 25, 2015
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@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@puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals


_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals
May 25, 2015
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@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@puremagic.com
>> http://lists.puremagic.com/mailman/listinfo/dmd-internals
>
>
> _______________________________________________
> dmd-internals mailing list
> dmd-internals@puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals
>
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals
May 25, 2015
On Monday, May 25, 2015 14:28:28 Steven Schveighoffer via dmd-internals wrote:
>  Let’s have a beer in Utah and get our goals all aligned :)

Now, for that, do you put the align directive on the struct or on a member variable? ;)

- Jonathan M Davis


_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals
« First   ‹ Prev
1 2 3