Walter, as gatekeeper to dmd, this is primarily addressed to you.  A major portion of engaging with a community of developers is being responsive to their desires to help out.  One of the primary reasons we switched to github was exactly to better engage with the community.  I really think taking more than a week to respond to a pull request is excessive.  Over a month, rather rude.  It's certainly not encouraging anyone to continue to invest their time.

So, would you take some time this weekend to catch up?  If there's something that you'd like either Don or I to do with them, speak up.  I'm willing to invest a bit of time if it'd help get the change merged in a more timely manner.

Here's the open pull requests for dmd:

  https://github.com/D-Programming-Language/dmd/pull/3  Feb 6, 2011
    const(Object) ref

  https://github.com/D-Programming-Language/dmd/pull/5  Feb 9, 2011
    Issue 4360 - Allow intrinsics in core.bitop to operate as intrinsics

  https://github.com/D-Programming-Language/dmd/pull/6  Feb 13, 2011
    Issue 4833 - map file location obeys -od/-of flags

  https://github.com/D-Programming-Language/dmd/pull/7  Feb 13, 2011
    Remove old druntime interface

  https://github.com/D-Programming-Language/dmd/pull/10 Feb 20, 2011
    Clarify tuple length error message

  https://github.com/D-Programming-Language/dmd/pull/11 Feb 26, 2011
    Issue 3541 - Add -oq to dmd (use fully qualified module name as object filename)

  https://github.com/D-Programming-Language/dmd/pull/12 Mar 3, 2011
    Bug fixes I threw patches in the past
    (NOTE: looks like at least part of this one needs to be delayed.  Thanks for reviewing it Don!)

If you don't think that the code is ready to be pulled, at a minimum, a comment needs to be added about what needs to be done.

Thanks,
Brad

An HTML attachment was scrubbed...
URL: <http://lists.puremagic.com/pipermail/dmd-internals/attachments/20110305/c3520a75/attachment.html>

Short of having more Don's, is there anything that can be done to reduce how much time it takes to process patches?

Automated test suite against dmd with the patch? Detection of patches that apply cleanly?

A less automated thing is someone to check if patches come with new tests that verify they do what is intended.

Etc...

Sent from my iPhone

On Mar 6, 2011, at 1:43 AM, Walter Bright <walter at digitalmars.com> wrote:

> I know, I know, but this piles up faster and faster. 8 new bugzilla reports just today. Many come with patches. I fixed one.
> 
> Brad Roberts wrote:
>> 
>> Walter, as gatekeeper to dmd, this is primarily addressed to you.  A major portion of engaging with a community of developers is being responsive to their desires to help out.  One of the primary reasons we switched to github was exactly to better engage with the community.  I really think taking more than a week to respond to a pull request is excessive.  Over a month, rather rude.  It's certainly not encouraging anyone to continue to invest their time.
>> 
>> So, would you take some time this weekend to catch up?  If there's something that you'd like either Don or I to do with them, speak up.  I'm willing to invest a bit of time if it'd help get the change merged in a more timely manner.
>> 
>> Here's the open pull requests for dmd:
>> 
>>   https://github.com/D-Programming-Language/dmd/pull/3  Feb 6, 2011
>>     const(Object) ref
>> 
>>   https://github.com/D-Programming-Language/dmd/pull/5  Feb 9, 2011
>>     Issue 4360 - Allow intrinsics in core.bitop to operate as intrinsics
>> 
>>   https://github.com/D-Programming-Language/dmd/pull/6  Feb 13, 2011
>>     Issue 4833 - map file location obeys -od/-of flags
>> 
>>   https://github.com/D-Programming-Language/dmd/pull/7  Feb 13, 2011
>>     Remove old druntime interface
>> 
>>   https://github.com/D-Programming-Language/dmd/pull/10 Feb 20, 2011
>>     Clarify tuple length error message
>> 
>>   https://github.com/D-Programming-Language/dmd/pull/11 Feb 26, 2011
>>     Issue 3541 - Add -oq to dmd (use fully qualified module name as object filename)
>> 
>>   https://github.com/D-Programming-Language/dmd/pull/12 Mar 3, 2011
>>     Bug fixes I threw patches in the past
>>     (NOTE: looks like at least part of this one needs to be delayed.  Thanks for reviewing it Don!)
>> 
>> If you don't think that the code is ready to be pulled, at a minimum, a comment needs to be added about what needs to be done.
>> 
>> Thanks,
>> Brad
>> _______________________________________________
>> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.puremagic.com/pipermail/dmd-internals/attachments/20110306/416f9e89/attachment.html>

On 6 March 2011 07:43, Walter Bright <walter at digitalmars.com> wrote:
> I know, I know, but this piles up faster and faster. 8 new bugzilla reports just today. Many come with patches. I fixed one.

Does everyone know about github's Fork Queue? I think it is excellent.
It is particularly great for cherry-picking commits.
(Eg, if everything in a pull request is trivially correct, except one
which needs more thought, use it to merge in everything except the
difficult one.
Takes just a few seconds).

Let's try to analyze the problem a bit more. Where is bottleneck,
exactly? Linus Torvalds manages to merge an incredible number of
patches per year.
So I don't think the sheer number of patches themselves is the issue.

Is it evaluating them? If so, it that because the patches don't have
adequate explanation?
Is it because you don't have evidence to trust the contributor has
enough knowledge to make a correct patch? This could be partially
overcome by having a regular reliable patcher (eg, me or Rainer) check
them first. Maybe there's other things we can do.


But if the bottleneck isn't the evaluation, then I think you're just
doing too much work. You  just need to use a binary approach: accept
without modifications, or reject.
EG.  Looks correct but... Please add unit tests. Please include D1
version. Please fix formatting.

Because really, if a patch is OK, the actions can be as simple as:
- merge with ForkQueue.
- switch to dmd1-x branch. Merge with ForkQueue.

Then after integrating a huge raft of changes, if all tests pass, add to the changelog. If a failure occurs, revert them.

Pull requester can be responsible for marking Bugzilla with the fix.

If you integrate good patches very quickly, it'll be OK to be have fairly harsh rules about them.

OTOH I don't know what to about the const(Object) ref pull request. That will definitely take a long time to evaluate.

Don Clugston wrote:
> On 6 March 2011 07:43, Walter Bright <walter at digitalmars.com> wrote:
> 
>> I know, I know, but this piles up faster and faster. 8 new bugzilla reports
>> just today. Many come with patches. I fixed one.
>> 
>
> Does everyone know about github's Fork Queue? I think it is excellent.
> It is particularly great for cherry-picking commits.
> (Eg, if everything in a pull request is trivially correct, except one
> which needs more thought, use it to merge in everything except the
> difficult one.
> Takes just a few seconds).
>
> Let's try to analyze the problem a bit more. Where is bottleneck,
> exactly? Linus Torvalds manages to merge an incredible number of
> patches per year.
> So I don't think the sheer number of patches themselves is the issue.
>
> Is it evaluating them? If so, it that because the patches don't have
> adequate explanation?
> Is it because you don't have evidence to trust the contributor has
> enough knowledge to make a correct patch? This could be partially
> overcome by having a regular reliable patcher (eg, me or Rainer) check
> them first. Maybe there's other things we can do.
>
>
> But if the bottleneck isn't the evaluation, then I think you're just
> doing too much work. You  just need to use a binary approach: accept
> without modifications, or reject.
> EG.  Looks correct but... Please add unit tests. Please include D1
> version. Please fix formatting.
>
> Because really, if a patch is OK, the actions can be as simple as:
> - merge with ForkQueue.
> - switch to dmd1-x branch. Merge with ForkQueue.
>
> Then after integrating a huge raft of changes, if all tests pass, add to the changelog. If a failure occurs, revert them.
>
> Pull requester can be responsible for marking Bugzilla with the fix.
>
> If you integrate good patches very quickly, it'll be OK to be have fairly harsh rules about them.
>
> OTOH I don't know what to about the const(Object) ref pull request.
> That will definitely take a long time to evaluate.
> 

One problem is evaluating what should be done about D1. I'll evaluate if and how they should be folded into D1, then merge, test with D1, update D1 test suite, etc. Not a huge problem, but time consuming.

(With your last patch list, I just could not get the merge to work automatically for D1, I went and did it manually.)

Probably the largest problem is just the context switch. When I'm working on, say, 64 bit code gen it's hard to look at other parts of the code. Right now I'm trying to figure out how to integrate in destructor calls for temporaries.

On 03/06/2011 07:20 AM, Jason House wrote:
> Short of having more Don's, is there anything that can be done to reduce how much time it takes to process patches?
>
> Automated test suite against dmd with the patch? Detection of patches that apply cleanly?
>
> A less automated thing is someone to check if patches come with new tests that verify they do what is intended.
>

Even that could be automated to some extent:

- apply any part of the patch that updates the tests
- run the tests
- apply the rest
- run the test again

If any test go from pass to fail, flag that. If no test go from fail to pass, flag that.

This wouldn't work with all requests, but some sort of annotation standard for the comments would allow for overriding as well as automation of things like updating bugzilla items.

> Etc...
>
> Sent from my iPhone
>
> On Mar 6, 2011, at 1:43 AM, Walter Bright <walter at digitalmars.com <mailto:walter at digitalmars.com>> wrote:
>
>> I know, I know, but this piles up faster and faster. 8 new bugzilla reports *just today*. Many come with patches. I fixed one.
>>
>> Brad Roberts wrote:
>>> Walter, as gatekeeper to dmd, this is primarily addressed to you.  A major portion of engaging with a community of developers is being responsive to their desires to help out.  One of the primary reasons we switched to github was exactly to better engage with the community.  I really think taking more than a week to respond to a pull request is excessive.  Over a month, rather rude.  It's certainly not encouraging anyone to continue to invest their time.
>>>
>>> So, would you take some time this weekend to catch up?  If there's something that you'd like either Don or I to do with them, speak up.  I'm willing to invest a bit of time if it'd help get the change merged in a more timely manner.
>>>
>>> Here's the open pull requests for dmd:
>>>
>>>    https://github.com/D-Programming-Language/dmd/pull/3   Feb 6, 2011
>>>      const(Object) ref
>>>
>>>    https://github.com/D-Programming-Language/dmd/pull/5   Feb 9, 2011
>>>      Issue 4360 - Allow intrinsics in core.bitop to operate as intrinsics
>>>
>>>    https://github.com/D-Programming-Language/dmd/pull/6   Feb 13, 2011
>>>      Issue 4833 - map file location obeys -od/-of flags
>>>
>>>    https://github.com/D-Programming-Language/dmd/pull/7   Feb 13, 2011
>>>      Remove old druntime interface
>>>
>>>    https://github.com/D-Programming-Language/dmd/pull/10  Feb 20, 2011
>>>      Clarify tuple length error message
>>>
>>>    https://github.com/D-Programming-Language/dmd/pull/11  Feb 26, 2011
>>>      Issue 3541 - Add -oq to dmd (use fully qualified module name as object filename)
>>>
>>>    https://github.com/D-Programming-Language/dmd/pull/12  Mar 3, 2011
>>>      Bug fixes I threw patches in the past
>>>      (NOTE: looks like at least part of this one needs to be delayed.  Thanks for reviewing it Don!)
>>>
>>> If you don't think that the code is ready to be pulled, at a minimum, a comment needs to be added about what needs to be done.
>>>
>>> Thanks,
>>> Brad
>>> _______________________________________________
>>> dmd-internals mailing list
>>> dmd-internals at puremagic.com  <mailto:dmd-internals at puremagic.com>
>>> http://lists.puremagic.com/mailman/listinfo/dmd-internals
>>>
>>>
>>> 
>> _______________________________________________
>> dmd-internals mailing list
>> dmd-internals at puremagic.com <mailto: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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.puremagic.com/pipermail/dmd-internals/attachments/20110306/867da692/attachment.html>

On 03/06/2011 11:49 AM, Walter Bright wrote:
>
>
> Don Clugston wrote:
>> On 6 March 2011 07:43, Walter Bright <walter at digitalmars.com> wrote:
>>> I know, I know, but this piles up faster and faster. 8 new bugzilla
>>> reports
>>> just today. Many come with patches. I fixed one.
>>
>> Does everyone know about github's Fork Queue? I think it is excellent.
>> It is particularly great for cherry-picking commits.
>> (Eg, if everything in a pull request is trivially correct, except one
>> which needs more thought, use it to merge in everything except the
>> difficult one.
>> Takes just a few seconds).
>>
>> Let's try to analyze the problem a bit more. Where is bottleneck,
>> exactly? Linus Torvalds manages to merge an incredible number of
>> patches per year.
>> So I don't think the sheer number of patches themselves is the issue.
>>
>> Is it evaluating them? If so, it that because the patches don't have
>> adequate explanation?
>> Is it because you don't have evidence to trust the contributor has
>> enough knowledge to make a correct patch? This could be partially
>> overcome by having a regular reliable patcher (eg, me or Rainer) check
>> them first. Maybe there's other things we can do.
>>
>>
>> But if the bottleneck isn't the evaluation, then I think you're just
>> doing too much work. You  just need to use a binary approach: accept
>> without modifications, or reject.
>> EG.  Looks correct but... Please add unit tests. Please include D1
>> version. Please fix formatting.
>>
>> Because really, if a patch is OK, the actions can be as simple as:
>> - merge with ForkQueue.
>> - switch to dmd1-x branch. Merge with ForkQueue.
>>
>> Then after integrating a huge raft of changes, if all tests pass, add to the changelog. If a failure occurs, revert them.
>>
>> Pull requester can be responsible for marking Bugzilla with the fix.
>>
>> If you integrate good patches very quickly, it'll be OK to be have fairly harsh rules about them.
>>
>> OTOH I don't know what to about the const(Object) ref pull request. That will definitely take a long time to evaluate.
>
> One problem is evaluating what should be done about D1. I'll evaluate if and how they should be folded into D1, then merge, test with D1, update D1 test suite, etc. Not a huge problem, but time consuming.
>
> (With your last patch list, I just could not get the merge to work automatically for D1, I went and did it manually.)
>
> Probably the largest problem is just the context switch. When I'm working on, say, 64 bit code gen it's hard to look at other parts of the code. Right now I'm trying to figure out how to integrate in destructor calls for temporaries.

A bit more visibility into the process might help the situation. Even just flagging anything you look at with boiler plate comments might do it. If nothing else,  no news, could be good news.

On Mar 6, 2011, at 2:10 PM, Don Clugston wrote:
> 
> I don't know what to about the const(Object) ref pull request. That will definitely take a long time to evaluate.

If unit test coverage is reasonable, has good code style, and does not break existing tests, why not issue provisional acceptance? Merge the code, possibly with #ifdef's. If bugs with it pop up in the next few months, ask the original author to provide patches. If they don't provide patches, remove it for good and give strict criteria on future attempts to add the feature.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.puremagic.com/pipermail/dmd-internals/attachments/20110306/67b26672/attachment.html>

Le 2011-03-06 ? 14:10, Don Clugston a ?crit :

> OTOH I don't know what to about the const(Object) ref pull request. That will definitely take a long time to evaluate.

Indeed, and I don't expect it to be integrated as fast as a normal pull request. It's fine for a more complicated patch to take longer to review. Don't hesitate to merge smaller ones first even if mine is older. Still, I hope someone looks at it eventually.

Also, this pull request is small relatively to what I'm preparing regarding integration of the Objective-C object model as extern(Objective-C). In a way you could say I'm testing the process with this smaller one first.

-- 
Michel Fortin
michel.fortin at michelf.com
http://michelf.com/

On 03/06/2011 01:14 PM, Michel Fortin wrote:
> Le 2011-03-06 ? 14:10, Don Clugston a ?crit :
>
>> OTOH I don't know what to about the const(Object) ref pull request. That will definitely take a long time to evaluate.
> Indeed, and I don't expect it to be integrated as fast as a normal pull request. It's fine for a more complicated patch to take longer to review. Don't hesitate to merge smaller ones first even if mine is older. Still, I hope someone looks at it eventually.
>
> Also, this pull request is small relatively to what I'm preparing regarding integration of the Objective-C object model as extern(Objective-C). In a way you could say I'm testing the process with this smaller one first.

Given that we now have three(?) extern(Foo)'s in play (if not in the official version), should the API of what an extern looks like be formalised? Might it even be done so that unofficial support for other languages could be implemented as SO/DLL's without needing to rebuild dmd?