[Mono-dev] Pull Requests

Thu Oct 16 22:12:13 UTC 2014

Hi Miguel,

I have considered helping out by commenting on PR's, however, I've always
felt that I'm not nearly experienced enough to have my opinion/view
respected by the committer.

I definitely think forcing/gently encouraging people to create a discussion
on mailing list to ask for review would help.  Maybe a format for the
subject? i.e. PR#1123 - <short description>.  this would help people filter
if they so desire.

I think if that is the position though, there needs to be a way to say
"non-contributors have said it's ok, let's get a proper review now".  I'm
not sure how you could do that though, is there a workflow in GitHub that
could help? maybe something you have at Xamarin that could be re-used?

I'm going to try and help out with a few this week and we'll see how it
goes.

Thanks,
Martin.

On 16 October 2014 22:28, Miguel de Icaza <miguel at xamarin.com> wrote:

> Let me add a couple of points.
>
> First, I have noticed is that:
>
>    - Contributors do not make a habit out of checking pull requests;   I
>    know I don't
>
>    - GitHub is too chatty, so everyone I know just filters notifications.
>      I suspect this is why anyone being assigned issues just ignores them.
>    There is just too much traffic.
>
> Mono historically took patches from the mailing list.   Not from github
> pull requests.   So culturally, this was never aligned, it just kind of
> happened, and we kind of never evolved or changed with it.
>
> Perhaps what we need is for these pull requests to be posted here on the
> list and discussed here on the list.
>
> I think this is better, as we have a place where the larger community can
> comment on patches.   It would surface the discussion to everyone, as
> opposed to limiting it to those that happen to stumble on the pull requests
> or visit the page.
>
> Second, I think there is a room for folks to contribute to this process,
> even if they are not committers, or active contributors.  Someone that
> could shepherd a contributor and the contribution.   Many of the patches do
> not meet the basic requirements for a patch review, and we end up wasting
> valuable time on them.
>
> Things that we would need:
>
>    - An actual detailed explanation of the change.   Many patches are
>    submitted with very little information.
>
>    - Test cases: many patches are contributed without tests to show the
>    problem as it is today nor to ensure that the code does not regress.
>
>    - Style: many patches contain white space changes, unnecessary
>    refactoring and changes that are not suitable to be contributed.
>
>    - Rebasing: many patches are contributed and fixes are piled on top of
>    each other.   The result is not suitable to be merged as it would
>    essentially pollute the commit history.   So someone that can assist the
>    less sophisticated among us to "squash" the commits would help.
>
>    - Patches that change the surface and contain no documentation.
>
>    - Steering developers to discuss the patches on the mailing list and
>    following up directly on the mailing list with the reviewers to ensure that
>    the patches can be merged.
>
> I think the above would help us tremendously to accelerate the patch
> review process.
>
> Miguel
>
> On Thu, Oct 16, 2014 at 4:30 PM, Miguel de Icaza <miguel at xamarin.com>
> wrote:
>
>> There is no point in starting a discussion where you are going to cherry
>> pick facts for the sake of your argument.
>>
>> As for contributing, which one of *your* pull requests have been pending
>> and not being reviewed?
>>
>> Because we would like to provide you with the valuable feedback that you
>> need to turn these contributions into patches.
>>
>> Miguel
>>
>> On Thu, Oct 16, 2014 at 4:25 PM, David Nelson <
>> eatdrinksleepcode at gmail.com> wrote:
>>
>>> "Long term, the ideal situation is one where we can give more people
>>> commit rights, and review rights.   But until we have developed the skills
>>> in the community that are needed, we will continue with the current setup."
>>>
>>> This seems to be a chicken-and-egg problem. We need to christen more
>>> reviewers in order to handle the volume of PRs and keep the Mono community
>>> engaged; but in order to gain enough confidence in a contributor to make
>>> them a reviewer, their requests need to be reviewed! How can we "develop
>>> the skills in the community" if requests routinely sit idle for over a year?
>>>
>>> I got really excited about contributing to Mono about two years ago; I
>>> love .NET and C#, but many of my colleagues (not to mention many of the
>>> companies for which we consult) are staunchly anti-Windows; I wanted to
>>> help demonstrate that Mono could be a viable alternative for non-Windows
>>> development. But research into the state of the community left me
>>> disappointed: PRs are ignored, roadmaps are horribly out of date, builds
>>> are constantly broken...in general, not an environment that encourages
>>> community members to contribute their valuable time.
>>>
>>> I understand the desire to maintain a high standard for contributed
>>> code, and I support maintaining that standard; but a process MUST be
>>> developed that encourages community contribution rather than stagnating it.
>>>
>>> On Thu, Oct 16, 2014 at 3:31 PM, Miguel de Icaza <miguel at xamarin.com>
>>> wrote:
>>>
>>>> Hello Greg,
>>>>
>>>> The best approach is to stay engaged in the pull requests and bring the
>>>> attention to the mailing list for us to discuss.
>>>>
>>>> Long term, the ideal situation is one where we can give more people
>>>> commit rights, and review rights.   But until we have developed the skills
>>>> in the community that are needed, we will continue with the current setup.
>>>>
>>>> The bar for mono is high: we can not just take any code and distribute
>>>> it, since the impact of mistakes is large.
>>>>
>>>> To give an example, even new Xamarin employees that are hired to work
>>>> exclusively on the runtime are working through pull requests, and they also
>>>> have to wait for some of the more senior people to review and approve the
>>>> patches.   We have very nice fixes that we still postpone until we have the
>>>> bandwidth of doing a full review.
>>>>
>>>> In the meantime, if you need quick hacks, you can always fork Mono and
>>>> distribute your forked version with your changes.
>>>>
>>>> Miguel
>>>>
>>>> On Thu, Oct 16, 2014 at 3:27 PM, Greg Young <gregoryyoung1 at gmail.com>
>>>> wrote:
>>>>
>>>>> This topic has been brought up in a ton of other threads I just want
>>>>> to centralize the topic.
>>>>>
>>>>> I have felt the pain many others have discussed (6-12 months for an
>>>>> accept of PR, we actually had a separate distribution of mono for a
>>>>> while).
>>>>>
>>>>> Is there background on the issue?
>>>>> What are the issues that are involved from a xamarin perspective?
>>>>> How can the community help?
>>>>>
>>>>> Cheers,
>>>>>
>>>>> Greg
>>>>>
>>>>> --
>>>>> Studying for the Turing test
>>>>> _______________________________________________
>>>>> Mono-devel-list mailing list
>>>>> Mono-devel-list at lists.ximian.com
>>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Mono-devel-list mailing list
>>>> Mono-devel-list at lists.ximian.com
>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>
>>>>
>>>
>>
>
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ximian.com/pipermail/mono-devel-list/attachments/20141016/65e8277c/attachment-0001.html>