On Aug 21, 2012, at 09:43 AM, Don Clugston <dclugston@gmail.com> wrote:

> On the matter of the 100 open pull requests -- I don't have any idea how we can deal with this. More people would help with the issue mentioned in the original post (which is basically that Kenji is a prolific author!), but wouldn't reduce the number of open pull requests in general. It seems to me to be an inevitable consequence of GitHub, where we're using a single list for both 'work in progress' and 'ready for review'.

On github an issue is automatically created when a pull request is made. Issues can be marked with different flags of different colors. But since we don't use the github issues it makes it harder. An issue with a corresponding pull request is labeled with "Code Attached". Perhaps we can start using github issues but just as a way to label pull requests?

--
/Jacob Carlborg

On 21 August 2012 10:06, Jonathan M Davis <jmdavisProg@gmx.com> wrote:
> On Tuesday, August 21, 2012 09:43:52 Don Clugston wrote:
>>  As far as I can tell, you can't even get the single most
>> important piece of information: which pull requests have I made (for
>> all projects), and how many are still open?
>
> You can easily get at the list of pull requests that you currently have open across all projects on github by clicking on "Pull Requests" on the main page. You can also get the list of _all_ of the pull requests that have been closed (be it because they were merged or just closed) by clicking on the "Closed" button on that pull requests page.
>
> - Jonathan M Davis

I don't see it. The only time I see the 'pull requests' button is for
an individual project, and that is "requests received" rather than
"requests sent".
What do you mean by the "main page"? Could you give me a URL of a page
where it is visible?
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

On Tuesday, August 21, 2012 10:24:53 Don Clugston wrote:
> On 21 August 2012 10:06, Jonathan M Davis <jmdavisProg@gmx.com> wrote:
> > On Tuesday, August 21, 2012 09:43:52 Don Clugston wrote:
> >>  As far as I can tell, you can't even get the single most
> >> 
> >> important piece of information: which pull requests have I made (for all projects), and how many are still open?
> > 
> > You can easily get at the list of pull requests that you currently have open across all projects on github by clicking on "Pull Requests" on the main page. You can also get the list of _all_ of the pull requests that have been closed (be it because they were merged or just closed) by clicking on the "Closed" button on that pull requests page.
> > 
> > - Jonathan M Davis
> 
> I don't see it. The only time I see the 'pull requests' button is for
> an individual project, and that is "requests received" rather than
> "requests sent".
> What do you mean by the "main page"? Could you give me a URL of a page
> where it is visible?

http://github.com

If you're logged in, you should see a bar near the top which lists

News Feed
Your Actions
Pull Requests
Issues
Stars

If you click on the Pull Requests button, you'll get to the page that lists all of your pull requests.

- Jonathan M Daviz
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

On 21 August 2012 10:29, Jonathan M Davis <jmdavisProg@gmx.com> wrote:
> On Tuesday, August 21, 2012 10:24:53 Don Clugston wrote:
>> On 21 August 2012 10:06, Jonathan M Davis <jmdavisProg@gmx.com> wrote:
>> > On Tuesday, August 21, 2012 09:43:52 Don Clugston wrote:
>> >>  As far as I can tell, you can't even get the single most
>> >>
>> >> important piece of information: which pull requests have I made (for all projects), and how many are still open?
>> >
>> > You can easily get at the list of pull requests that you currently have open across all projects on github by clicking on "Pull Requests" on the main page. You can also get the list of _all_ of the pull requests that have been closed (be it because they were merged or just closed) by clicking on the "Closed" button on that pull requests page.
>> >
>> > - Jonathan M Davis
>>
>> I don't see it. The only time I see the 'pull requests' button is for
>> an individual project, and that is "requests received" rather than
>> "requests sent".
>> What do you mean by the "main page"? Could you give me a URL of a page
>> where it is visible?
>
> http://github.com
>
> If you're logged in, you should see a bar near the top which lists
>
> News Feed
> Your Actions
> Pull Requests
> Issues
> Stars

Thanks, I never saw that page before. I've only used my page
(github.com/donc), which is total garbage.
I withdraw one of my 50 complaints about github.
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

Andrei Alexandrescu, el 20 de August a las 14:39 me escribiste:
> On 8/20/12 2:31 PM, David Nadlinger wrote:
> >On Mon, Aug 20, 2012 at 8:09 PM, Brad Roberts<braddr@puremagic.com>  wrote:
> >>Sorry, but the fix to not enough people with front end experience doing code reviews is not to not do code reviews.
> >
> >Agreed, but I'd like to add that this is a general problem with DMD development (maintainability?) - even Walter regularly commits code which doesn't even pass the auto tester.
>
> This issue is a symptom of a simple underlying problem: we don't have enough contributors. Kenji tried to find a solution within the current constraints, whereas Brad responded from an "enough reviewers" frame of reference.
>
> I think we need to add more active committers to dmd.

That's not really necessary, all you need is Walter trusting other people and then *pulling* from other people's repos. That way he can even verify the changes before *pushing* to the public repo. The workflow for Walter can be:

1) pull from trusted contributor
2) verify (run the tests, locally review the changes, maybe just read
   the commit title to see what kind of changes are being pulled, or
   whatever, including doing nothing)
3) pushing to the public repo (or not, if 2) have problems)

This greatly reduces Walter's job, but he can still have full control on
*when* things get pushed to the public repo. For example, when
a stabilization process starts, is not so important to ask the
contributors to stop pushing big changes, everyone can keep doing
whatever they want without messing with the public repo.

I think the main problem here is Walter having to review and test every single commit. If he trust contributor, he can pull from then and introduce a batch of commits with just a minimal verification process for the whole batch, instead of having to go through them one by one. For me having a lot of people pushing to a repo is too risky, to easy to make a mess of it (BTW this is how Linux development process works, only Linus pushes to the public vanilla kernel repo, but he pulls from people he trust, which in turn pull from the people they trust and that's how the model scales up with a "tree of trust").

--
Leandro Lucarella (AKA luca)                     http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
Y tuve amores, que fue uno sólo
El que me dejó de a pie y me enseñó todo...Bien
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

kenji hara, el 20 de August a las 11:40 me escribiste:
> I had been invited as dmd committer a month ago, but haven't been able to consume pull requests.
>
> The top three of pulls at 2012/08/20:
>   37 pulls : my front-end changes
>   17 pulls : yebblies's front-end changes
>   13 pulls : dawgfoto's backend and front-end changes
> -> 67/103 (65% of all pulls)
> and, as far as I seen half of them are trivial bug fixes and LGTM.
>
> But they are yet not merged by the following reasons.
> #1. Some of them are mine, so I cannot merge without others reviewing.
> #2. Some of them are already outdated, so I cannot merge them without
> small fix-up.
>     But, even if I add some fix-ups to them, I can not merge them by
> the reason #1 .

I think ideally somebody should review your commits, but if you've been offered commit rights, then it's implied you don't *need* review (nobody reviews Walter commits, not at least before they are pushed to the repo). Again, it would be ideal if somebody else review the patch, but I don't think it should be a blocker.

But that's just *my* POV and I have no idea what does it mean for Walter to give commit rights (I guess is push rights in git terminology).

--
Leandro Lucarella (AKA luca)                     http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
Are you such a dreamer?
To put the world to rights?
I'll stay home forever
Where two & two always
makes up five
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

On 8/21/12 9:21 AM, Leandro Lucarella wrote:
> I think ideally somebody should review your commits, but if you've been
> offered commit rights, then it's implied you don't *need* review (nobody
> reviews Walter commits, not at least before they are pushed to the
> repo). Again, it would be ideal if somebody else review the patch, but
> I don't think it should be a blocker.
>
> But that's just *my* POV and I have no idea what does it mean for Walter
> to give commit rights (I guess is push rights in git terminology).

The way it works for us at Facebook is that every line of code must be reviewed by another engineer than the one who wrote it. It is working very well for us.

There are many positive consequences to this. If the rule were not in effect, I suspect the notion of "this code is mine" and "that code is yours" would immediately emerge. With peer review enforced it becomes clear that once a pull request is merged, it is owned by the entire team.


Andrei
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

On 8/21/12 9:20 AM, Leandro Lucarella wrote:
> I think the main problem here is Walter having to review and test every
> single commit.

This has not been the case for a short while.

Andrei
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

On Tue, Aug 21, 2012 at 4:53 PM, Andrei Alexandrescu <andrei@erdani.com> wrote:
> There are many positive consequences to this. If the rule were not in effect, I suspect the notion of "this code is mine" and "that code is yours" would immediately emerge. With peer review enforced it becomes clear that once a pull request is merged, it is owned by the entire team.

Which is, incidentally, one of the reasons why I'm not particularly fond of author tags in source files. If you want to know who actually made the commits, just look at the SCM history…

David
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

Another proposal: introduce beta repository similar to linux-next,
where pull request
may be accepted without full code review. At some time when new
version is being
prepared commits which are considered to be appropriate can be pulled
to official
repository. It doesn't mean that not reviewed patches would get to upstream.
Auto-tester needs to be configured to work with beta too.

Pros:
- addresses out-of-date issue mentioned by Kenji Hara in first message
- "time" test: if patch was in beta tree for several months and
doesn't contain obvious
problems, than it is likely to be correct (assumption here is that
beta is tested enough),
even if this not always hold commits are at least tested by group of
developers for a while
- patches which are sent to upstream are still reviewed as much as necessary, so
  no harm done to upstream code quality
- patches in pull request queue may live for several month, if they
are kept in beta
  tree it is not worse than keeping them in pull requests (and I
believe, actually better)
- commits may be tested not individually, but in large groups within
entire test tree
- if patch is pushed to beta, it forces all participating developers
who sync their
  repositories to test it while working on their projects (as
mentioned, here is
  implicit assumption about sync frequency and something else)
- commits can reviewed not only individually but in groups
- it easier to fix pushed commit than pull request

Cons:
- still need to review
- new problem - resolving conflicts between upstream and beta
- more commits are pushed to beta - more review work
- breaking code may occur in test branch/repo

Anyway, many projects contain beta or "rc" branches/repos. Dmd as I know has
some kind of this but at a limited period of time when making new version. So,
master branch/repo currently plays role of both test and release tree. Why not
split these roles?
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals