DMD backend quality (Was: Re: DIP 1031--Deprecate Brace-Style Struct Initializers--Community Review Round 1 Discussion)

jxel jxel at gmall.com
Thu Feb 20 15:47:51 UTC 2020


On Thursday, 20 February 2020 at 10:58:36 UTC, Walter Bright 
wrote:
> On 2/19/2020 10:34 PM, Seb wrote:
>> And I haven't even mentioned the bottleneck of any major DMD 
>> PR needing to be approved by Walter.
>
> Given the current regression list:
>
> https://issues.dlang.org/buglist.cgi?bug_severity=regression&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&list_id=230215&query_format=advanced
>
> I'm probably being too permissive. I'm usually the one who gets 
> to fix them. I asked in this forum for some help isolating 
> which PRs caused the regressions, to no avail.
>
> Major PRs *should* be regarded with skepticism. For example, I 
> recently fixed a regression caused by a 2 or 3 hundred line PR, 
> where the substance of the PR was one line and the rest of it 
> was refactoring.
>
> Folks, with PRs, smaller, tightly focused PRs are better. Do 
> not:
>
> 1. lump multiple issues into one PR
>
> 2. include your favorite refactorings in with it
>
> 3. refactors must not change observable behavior
>
> Do:
>
> 1. minimize the PR even if the result of the PR suggests a 
> refactoring
>
> 2. do that refactor SEPARATELY AFTER THE PR WAS MERGED

I've fixed a regression caused by one of your PRs. It was a small 
change to the actual code, couldn't have been more than 10 lines, 
but it changed a few of the test case outputs. Funnily enough 
there were valid test cases with error messages that were just 
nuked over blindly (automatically probably). Errors happen, even 
for very simple things, if you don't take care it doesn't matter 
what the size of the PR is.

People have trouble getting one PR merged, and you want them to 
try and split them up and somehow have that be coordinated 
properly when the PR have to be merged in a certain order and 
across 3 different projects? Yah, no thanks.



More information about the Digitalmars-d mailing list