View mode: basic / threaded / horizontal-split · Log in · Help
June 21, 2012
Handling of compiler patches
For DMD in GitHub there are more than one hundred open pull 
requests (currently 111). So far people have created more than 
one thousand patches for DMD (currently 1022):

https://github.com/D-Programming-Language/dmd/pulls

Among the list of open pull requests there are some bugs and 
enhancement requests that I'd really like to see applied & fixed 
in not too much time.

I am not a compiler writer, but I have two suggestions for Walter 
(or at least to open a discussion, if the things I am saying are 
wrong):

1) Currently the patches that Walter applies written by other 
people come mostly from the most recent ones. So the list of the 
open pull requests is managed almost as a stack (last in - fist 
out). I think this is not good.

I suggest to do the opposite, this means to manage that list more 
like a queue (first in - first out), so I suggest Walter to look 
for patches to apply starting from the oldest ones. This has the 
advantage that the patches don't get too much old, so they rot 
less.

An additional strategy to choose what patches to apply is to 
choose first the ones that fix bugs that are most significant. 
Bugs are not all equal, many bugs don't cause significant 
problems, while others are quite annoying. Significant bugs often 
are fixed by small patches. I see lot of not so significant 
patches applied, while I see patches that fix bugs that I hit 
every other day sleep in that list for weeks or months. If you 
want I will list some significant patches currently open.


2) I suggest Walter to increase a little more the openness of the 
way the front-end is developed. There are many kinds of compiler 
patches:

- Simple localized change;
- Complex localized change;
- Widespread trivial changes;
- Widespread complex changes;
- Back-end changes Vs front-end changes;
- That fixes a well defined and localized bug;
- That introduces a new feature, or a small part of a new feature.

The harder patches are better left to Walter, but I suggest 
Walter to eventually let few other people merge the simpler 
patches, after a review by one or two more persons.

In large projects, with skilled helper people, the leader 
eventually must learn to trust other people and to  delegate some 
of the work, building a pyramid of trust. I think that if Hara 
writes a simple 5-lines long patch for the DMD front-end, and Don 
or someone else equally skilled reviews and accepts that patch, 
there is no need for Walter to merge that little patch himself.

Bye,
bearophile
June 21, 2012
Re: Handling of compiler patches
+1

On Thursday, 21 June 2012 at 07:40:35 UTC, bearophile wrote:
> For DMD in GitHub there are more than one hundred open pull 
> requests (currently 111). So far people have created more than 
> one thousand patches for DMD (currently 1022):
>
> https://github.com/D-Programming-Language/dmd/pulls
>
> Among the list of open pull requests there are some bugs and 
> enhancement requests that I'd really like to see applied & 
> fixed in not too much time.
>
> I am not a compiler writer, but I have two suggestions for 
> Walter (or at least to open a discussion, if the things I am 
> saying are wrong):
>
> 1) Currently the patches that Walter applies written by other 
> people come mostly from the most recent ones. So the list of 
> the open pull requests is managed almost as a stack (last in - 
> fist out). I think this is not good.
>
> I suggest to do the opposite, this means to manage that list 
> more like a queue (first in - first out), so I suggest Walter 
> to look for patches to apply starting from the oldest ones. 
> This has the advantage that the patches don't get too much old, 
> so they rot less.
>
> An additional strategy to choose what patches to apply is to 
> choose first the ones that fix bugs that are most significant. 
> Bugs are not all equal, many bugs don't cause significant 
> problems, while others are quite annoying. Significant bugs 
> often are fixed by small patches. I see lot of not so 
> significant patches applied, while I see patches that fix bugs 
> that I hit every other day sleep in that list for weeks or 
> months. If you want I will list some significant patches 
> currently open.
>
>
> 2) I suggest Walter to increase a little more the openness of 
> the way the front-end is developed. There are many kinds of 
> compiler patches:
>
> - Simple localized change;
> - Complex localized change;
> - Widespread trivial changes;
> - Widespread complex changes;
> - Back-end changes Vs front-end changes;
> - That fixes a well defined and localized bug;
> - That introduces a new feature, or a small part of a new 
> feature.
>
> The harder patches are better left to Walter, but I suggest 
> Walter to eventually let few other people merge the simpler 
> patches, after a review by one or two more persons.
>
> In large projects, with skilled helper people, the leader 
> eventually must learn to trust other people and to  delegate 
> some of the work, building a pyramid of trust. I think that if 
> Hara writes a simple 5-lines long patch for the DMD front-end, 
> and Don or someone else equally skilled reviews and accepts 
> that patch, there is no need for Walter to merge that little 
> patch himself.
>
> Bye,
> bearophile
June 21, 2012
Re: Handling of compiler patches
On 2012-06-21 09:57, Andrea Fontana wrote:
> +1
>
> On Thursday, 21 June 2012 at 07:40:35 UTC, bearophile wrote:
>> For DMD in GitHub there are more than one hundred open pull requests
>> (currently 111). So far people have created more than one thousand
>> patches for DMD (currently 1022):
>>
>> https://github.com/D-Programming-Language/dmd/pulls
>>
>> Among the list of open pull requests there are some bugs and
>> enhancement requests that I'd really like to see applied & fixed in
>> not too much time.

+1

-- 
/Jacob Carlborg
June 21, 2012
Re: Handling of compiler patches
> 1) Currently the patches that Walter applies written by other people  
> come mostly from the most recent ones. I think this is not good.

Yep, github's default sort method sucks.
They should really implement some sort of voting scheme or something like  
a "ready to merge" button for prolific contributors.

Interestingly enough they even seem to have a 'votes' field for pull  
requests internally (http://develop.github.com/p/pulls.html) but I haven't  
seen any voting system yet.


> I suggest to do the opposite, this means to manage that list more like a  
> queue (first in - first out), so I suggest Walter to look for patches to  
> apply starting from the oldest ones. This has the advantage that the  
> patches don't get too much old, so they rot less.

+1
It's just annoying having to keep pull requests up-to-date forever.

AND it definitely pisses contributors off. At least I won't submit  
anything anymore if the pulls just rot anyways.


Also I really wonder if Walter's using the autotester.
Most patches are ridiculously small and/or non-critical, so reviewing the  
code by hand can't take that long.
And if you properly use the autotester instead of manually running the  
testsuite to verify it doesn't break anything you should really be able to  
merge a whole bunch of pulls in no time.

Why is the process still so slow?
June 21, 2012
Re: Handling of compiler patches
On 21-Jun-12 11:40, bearophile wrote:
> For DMD in GitHub there are more than one hundred open pull requests
> (currently 111). So far people have created more than one thousand
> patches for DMD (currently 1022):
>
> https://github.com/D-Programming-Language/dmd/pulls
>
> Among the list of open pull requests there are some bugs and enhancement
> requests that I'd really like to see applied & fixed in not too much time.
>
> I am not a compiler writer, but I have two suggestions for Walter (or at
> least to open a discussion, if the things I am saying are wrong):
>
Instead of pushing Walter to work some other way to improve his 
bandwidth I'd suggest to implement proper caching first.

What I suggest is to finally organize multi-stage development process 
that loosely models military command chain :)

OK. Walter is the general at HQ (@ dlang master branch) no doubt. And 
some of core contributors are assigned as lieutenants for feature 
oriented branches.

The structure I imagine is:

Walter (master branch)
  |--- CTFE feature branch, Don Clugston
  |--- Backend bugfix & feature branch, Brad Roberts
  |--- Language bugfix & feature, Kenji Hara / Daniel Murphy
  |--- Miscellaneous & ICE bugs, Martin Nowak

(we can start with a few less or more of these)

*Keep in mind these assignments are purely my speculation. And sure 
thing it doesn't prevent anybody from working on "someone else" feature 
branch. All these assignments mean is responsibility for pulling stuff 
and forming separate (or on the contrary - cumulative) pulls on them for 
HQ. Also allows them to polish and refactor things on the way up.

Then the model works as follows:

HQ repo gets pull requests prepared  by lieutenants *only* thus 
*insuring* it's not some "for the **ck of it pull request" and lifting 
ton of work for reviewing it.

Pulls for features are targeted at specific repo thus *insuring* there 
is at least one "major" guy how it supposed to work in this field (and 
in the know of proper DMD's conventions). All dirty hack pulls get 
stopped/re-written somewhere here.

Also all of lieutenants repos should be kept rebased on top of master 
(HQ) at all times. Thus Walter may even choose to cherry pick some 
commits skipping the usual pull request process.

-- 
Dmitry Olshansky
June 22, 2012
Re: Handling of compiler patches
On Thu, 21 Jun 2012 10:51:03 -0700, Dmitry Olshansky  
<dmitry.olsh@gmail.com> wrote:

> On 21-Jun-12 11:40, bearophile wrote:
>> For DMD in GitHub there are more than one hundred open pull requests
>> (currently 111). So far people have created more than one thousand
>> patches for DMD (currently 1022):
>>
>> https://github.com/D-Programming-Language/dmd/pulls
>>
>> Among the list of open pull requests there are some bugs and enhancement
>> requests that I'd really like to see applied & fixed in not too much  
>> time.
>>
>> I am not a compiler writer, but I have two suggestions for Walter (or at
>> least to open a discussion, if the things I am saying are wrong):
>>
> Instead of pushing Walter to work some other way to improve his  
> bandwidth I'd suggest to implement proper caching first.
>
> What I suggest is to finally organize multi-stage development process  
> that loosely models military command chain :)
>
> OK. Walter is the general at HQ (@ dlang master branch) no doubt. And  
> some of core contributors are assigned as lieutenants for feature  
> oriented branches.
>
> The structure I imagine is:
>
> Walter (master branch)
>    |--- CTFE feature branch, Don Clugston
>    |--- Backend bugfix & feature branch, Brad Roberts
>    |--- Language bugfix & feature, Kenji Hara / Daniel Murphy
>    |--- Miscellaneous & ICE bugs, Martin Nowak
>
> (we can start with a few less or more of these)
>
> *Keep in mind these assignments are purely my speculation. And sure  
> thing it doesn't prevent anybody from working on "someone else" feature  
> branch. All these assignments mean is responsibility for pulling stuff  
> and forming separate (or on the contrary - cumulative) pulls on them for  
> HQ. Also allows them to polish and refactor things on the way up.
>
> Then the model works as follows:
>
> HQ repo gets pull requests prepared  by lieutenants *only* thus  
> *insuring* it's not some "for the **ck of it pull request" and lifting  
> ton of work for reviewing it.
>
> Pulls for features are targeted at specific repo thus *insuring* there  
> is at least one "major" guy how it supposed to work in this field (and  
> in the know of proper DMD's conventions). All dirty hack pulls get  
> stopped/re-written somewhere here.
>
> Also all of lieutenants repos should be kept rebased on top of master  
> (HQ) at all times. Thus Walter may even choose to cherry pick some  
> commits skipping the usual pull request process.
>

I like this. It neatly solves a whole lot of issues. Also, this is  
precisely how Linus INTENDED Git to be used.

-- 
Adam Wilson
IRC: LightBender
Project Coordinator
The Horizon Project
http://www.thehorizonproject.org/
Top | Discussion index | About this forum | D home