You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Julian Hyde <jh...@apache.org> on 2017/08/21 19:33:22 UTC

[DISCUSS] Reviewing pull requests

I was on vacation last week. Before I went on vacation I sent an email to this list[0] asking the community of committers to stay on top of pull requests. My rationale, not explicitly stated in the email but hopefully clear to everyone, is that we need to respond to contributors in a timely fashion, otherwise they will not join the community. “Timely” means a day or two, maximum.

Since I went on vacation 8 days ago, an existing PR was committed, but 10 PRs have been created[1], and only one of them received feedback [2]. (Thanks, Jesus, for the commit and the review.) Several JIRA cases have been logged, also.

I don’t think this is an acceptable amount of feedback to sustain the community.

The Calcite community is successful and growing, but it takes work to keep it going. I’m tired of being the main person who does that work. Committers and PMC members, you all benefit from the community. You (rightly) expect that your own PRs are reviewed and committed in a timely manner. How can I encourage you to take a greater share of the work? Do I need to take more vacation?

Julian

[0] http://mail-archives.apache.org/mod_mbox/calcite-dev/201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D%40apache.org%3E <http://mail-archives.apache.org/mod_mbox/calcite-dev/201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D@apache.org%3E> 

[1] https://github.com/apache/calcite/pulls <https://github.com/apache/calcite/pulls>

[2] https://github.com/apache/calcite/pull/523#pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412> 

Re: [DISCUSS] Reviewing pull requests

Posted by Gian Merlino <gi...@imply.io>.
We have a similar problem in the Druid community. PR review backlogs are
long and not all committers help out equally. We're grateful when anyone
does show an interest in developing the Druid community rather than just
using Druid or contributing patches. But that interest in the community is
not universal. I don't think there are easy solutions, since it's a free
rider problem at heart, and it's tough to force people to help out.

My experience in Druid is that productive reviewers are usually either
doing it out of love for the project, or because of corporate sponsorship
(i.e. they're doing it at work because community development is one of
their job responsibilities). I'm not sure there are any other ways to get
people to contribute, short of making it mandatory (you can't get your
patch reviewed unless you review someone else's) or at least a strong
community norm (community members hassle you if you don't help out). I
guess that starting this thread, Julian, may end up making this more of a
community norm.

Gian

On Thu, Aug 24, 2017 at 12:40 PM, Julian Hyde <jh...@apache.org> wrote:

> Thanks for your support, Michael.
>
> Not a peep from the rest of you. I suppose there’s no incentive to speak
> up while the free lunch keeps on coming.
>
> What should I do to get some assistance from the many people that benefit
> from this being a healthy community? Do I have to threaten to go on strike?
>
> Julian
>
>
>
> > On Aug 22, 2017, at 8:01 AM, Michael Mior <mm...@uwaterloo.ca> wrote:
> >
> > I agree this is an important problem to solve. The biggest barrier for me
> > is that I'm not familiar with most of the Calcite code base and I don't
> > really have the time to invest right now in order to become familiar to
> the
> > point I'd be comfortable reviewing the majority of PRs. I do have
> > notifications of PRs enabled for the repo and will try to make an effort
> to
> > review when I feel qualified.
> >
> > --
> > Michael Mior
> > mmior@apache.org <ma...@apache.org>
> >
> > 2017-08-21 15:33 GMT-04:00 Julian Hyde <jhyde@apache.org <mailto:
> jhyde@apache.org>>:
> >
> >> I was on vacation last week. Before I went on vacation I sent an email
> to
> >> this list[0] asking the community of committers to stay on top of pull
> >> requests. My rationale, not explicitly stated in the email but hopefully
> >> clear to everyone, is that we need to respond to contributors in a
> timely
> >> fashion, otherwise they will not join the community. “Timely” means a
> day
> >> or two, maximum.
> >>
> >> Since I went on vacation 8 days ago, an existing PR was committed, but
> 10
> >> PRs have been created[1], and only one of them received feedback [2].
> >> (Thanks, Jesus, for the commit and the review.) Several JIRA cases have
> >> been logged, also.
> >>
> >> I don’t think this is an acceptable amount of feedback to sustain the
> >> community.
> >>
> >> The Calcite community is successful and growing, but it takes work to
> keep
> >> it going. I’m tired of being the main person who does that work.
> Committers
> >> and PMC members, you all benefit from the community. You (rightly)
> expect
> >> that your own PRs are reviewed and committed in a timely manner. How
> can I
> >> encourage you to take a greater share of the work? Do I need to take
> more
> >> vacation?
> >>
> >> Julian
> >>
> >> [0] http://mail-archives.apache.org/mod_mbox/calcite-dev/
> >> 201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D%40apache.org%3E <
> >> http://mail-archives.apache.org/mod_mbox/calcite-dev/ <
> http://mail-archives.apache.org/mod_mbox/calcite-dev/>
> >> 201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D@apache.org <mailto:
> 3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D@apache.org>%3E>
> >>
> >> [1] https://github.com/apache/calcite/pulls <
> https://github.com/apache/calcite/pulls> <https://github.com/apache/ <
> https://github.com/apache/>
> >> calcite/pulls>
> >>
> >> [2] https://github.com/apache/calcite/pull/523#pullrequestreview
> -57275412 <https://github.com/apache/calcite/pull/523#pullrequestrevie
> w-57275412>
> >> <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412
> <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412>>
>
>

Re: [DISCUSS] Reviewing pull requests

Posted by Michael Mior <mm...@uwaterloo.ca>.
I already feel a responsibility for the Cassandra adapter since that's been
my primary contribution. Although it doesn't seem to be getting a lot of
use, I review any PR or JIRA that pops up for that.

--
Michael Mior
mmior@apache.org

2017-08-24 21:04 GMT-04:00 zhiqiang <zh...@apache.org>:

> I think we can assign calcite module to every PMC and Committer, each
> module has two or more owner, and the responsibility of every module owner
> is review PRs , Commit module codes , discuss issues or fetures in JIRA.
> So this can make PRs commit more fast.
>
>
> Regards
> Zhiqiang He
>
> -----Original Message-----
> From: Julian Hyde [mailto:jhyde@apache.org]
> Sent: Friday, August 25, 2017 5:41 AM
> To: dev@calcite.apache.org
> Subject: Re: [DISCUSS] Reviewing pull requests
>
> You never really know whether you have thought of everything. Even if the
> PR has added tests, and the tests pass, have they tested everything that
> needs to be tested? If there is an architectural change (and there almost
> always is some architectural impact, even if it is just doing “more of the
> same” in an architectural area that is getting a bit creaky) how much
> burden do we put on the contributor to solve it?
>
> The best you can do is make your best judgment call. I promise never to
> criticize someone who does what they think best, and I’m sure the other
> committers feel that way.
>
> That said, if you have carried out an initial review and you think a
> second opinion would help, just ask for it.
>
> And by the way, as a project we have never decided whether we were
> commit-then-review (CTR) or review-then-commit (RTC). The policy has been
> “do what you think best”, and in my opinion that policy is working. If you
> are a committer and you think your code doesn’t need review, just go ahead
> and commit it. I often do this, whereas most other committers usually seem
> to ask for review (this week I have reviewed PRs from Maryann and Minji,
> who are both committers), but I am not in a privileged position. Just use
> your discretion, and you won’t be blamed for it.
>
> Julian
>
>
> > On Aug 24, 2017, at 1:54 PM, Josh Elser <jo...@gmail.com> wrote:
> >
> > Sorry I didn't respond sooner (treading water at $dayjob to stay
> afloat). This is a good conversation that we should have as a community.
> >
> > I have to echo Michael's assessment of feeling "uncomfortable" reviewing
> many contributions due to lack of experience. I recognize that this is a
> bit of a fallacy, so, sorry I haven't been able to step up more.
> >
> > Abstractly, figuring out the right balance for trust in new
> contributions would help remove such a worry. For example, how much trust
> do I put in a contribution I might not fully understand if the tests pass?
> >
> > If we can get there, at least the only show-stopper is my free time ;)
> >
> > On 8/24/17 3:40 PM, Julian Hyde wrote:
> >> Thanks for your support, Michael.
> >> Not a peep from the rest of you. I suppose there’s no incentive to
> speak up while the free lunch keeps on coming.
> >> What should I do to get some assistance from the many people that
> benefit from this being a healthy community? Do I have to threaten to go on
> strike?
> >> Julian
> >>> On Aug 22, 2017, at 8:01 AM, Michael Mior <mm...@uwaterloo.ca> wrote:
> >>>
> >>> I agree this is an important problem to solve. The biggest barrier for
> me
> >>> is that I'm not familiar with most of the Calcite code base and I don't
> >>> really have the time to invest right now in order to become familiar
> to the
> >>> point I'd be comfortable reviewing the majority of PRs. I do have
> >>> notifications of PRs enabled for the repo and will try to make an
> effort to
> >>> review when I feel qualified.
> >>>
> >>> --
> >>> Michael Mior
> >>> mmior@apache.org <ma...@apache.org> <mailto:mmior@apache.org
> <ma...@apache.org>>
> >>>
> >>> 2017-08-21 15:33 GMT-04:00 Julian Hyde <jhyde@apache.org <mailto:
> jhyde@apache.org> <mailto:jhyde@apache.org <ma...@apache.org>>>:
> >>>
> >>>> I was on vacation last week. Before I went on vacation I sent an
> email to
> >>>> this list[0] asking the community of committers to stay on top of pull
> >>>> requests. My rationale, not explicitly stated in the email but
> hopefully
> >>>> clear to everyone, is that we need to respond to contributors in a
> timely
> >>>> fashion, otherwise they will not join the community. “Timely” means a
> day
> >>>> or two, maximum.
> >>>>
> >>>> Since I went on vacation 8 days ago, an existing PR was committed,
> but 10
> >>>> PRs have been created[1], and only one of them received feedback [2].
> >>>> (Thanks, Jesus, for the commit and the review.) Several JIRA cases
> have
> >>>> been logged, also.
> >>>>
> >>>> I don’t think this is an acceptable amount of feedback to sustain the
> >>>> community.
> >>>>
> >>>> The Calcite community is successful and growing, but it takes work to
> keep
> >>>> it going. I’m tired of being the main person who does that work.
> Committers
> >>>> and PMC members, you all benefit from the community. You (rightly)
> expect
> >>>> that your own PRs are reviewed and committed in a timely manner. How
> can I
> >>>> encourage you to take a greater share of the work? Do I need to take
> more
> >>>> vacation?
> >>>>
> >>>> Julian
> >>>>
> >>>> [0] http://mail-archives.apache.org/mod_mbox/calcite-dev/ <
> http://mail-archives.apache.org/mod_mbox/calcite-dev/>
> >>>> 201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D%40apache.org <
> http://40apache.org/>%3E <
> >>>> http://mail-archives.apache.org/mod_mbox/calcite-dev/ <
> http://mail-archives.apache.org/mod_mbox/calcite-dev/> <
> http://mail-archives.apache.org/mod_mbox/calcite-dev/ <
> http://mail-archives.apache.org/mod_mbox/calcite-dev/>>
> >>>> 201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D@apache.org
> <ma...@apache.org> <mailto:
> 3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D@apache.org <mailto:
> 3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D@apache.org>>%3E>
> >>>>
> >>>> [1] https://github.com/apache/calcite/pulls <
> https://github.com/apache/calcite/pulls> <https://github.com/apache/
> calcite/pulls <https://github.com/apache/calcite/pulls>> <
> https://github.com/apache/ <https://github.com/apache/> <
> https://github.com/apache/ <https://github.com/apache/>>
> >>>> calcite/pulls>
> >>>>
> >>>> [2] https://github.com/apache/calcite/pull/523#
> pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#
> pullrequestreview-57275412> <https://github.com/apache/calcite/pull/523#
> pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#
> pullrequestreview-57275412>>
> >>>> <https://github.com/apache/calcite/pull/523#
> pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#
> pullrequestreview-57275412> <https://github.com/apache/calcite/pull/523#
> pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#
> pullrequestreview-57275412>>>
>
>
>

RE: [DISCUSS] Reviewing pull requests

Posted by zhiqiang <zh...@apache.org>.
I think we can assign calcite module to every PMC and Committer, each module has two or more owner, and the responsibility of every module owner is review PRs , Commit module codes , discuss issues or fetures in JIRA. 
So this can make PRs commit more fast.


Regards
Zhiqiang He

-----Original Message-----
From: Julian Hyde [mailto:jhyde@apache.org] 
Sent: Friday, August 25, 2017 5:41 AM
To: dev@calcite.apache.org
Subject: Re: [DISCUSS] Reviewing pull requests

You never really know whether you have thought of everything. Even if the PR has added tests, and the tests pass, have they tested everything that needs to be tested? If there is an architectural change (and there almost always is some architectural impact, even if it is just doing “more of the same” in an architectural area that is getting a bit creaky) how much burden do we put on the contributor to solve it?

The best you can do is make your best judgment call. I promise never to criticize someone who does what they think best, and I’m sure the other committers feel that way.

That said, if you have carried out an initial review and you think a second opinion would help, just ask for it.

And by the way, as a project we have never decided whether we were commit-then-review (CTR) or review-then-commit (RTC). The policy has been “do what you think best”, and in my opinion that policy is working. If you are a committer and you think your code doesn’t need review, just go ahead and commit it. I often do this, whereas most other committers usually seem to ask for review (this week I have reviewed PRs from Maryann and Minji, who are both committers), but I am not in a privileged position. Just use your discretion, and you won’t be blamed for it.

Julian


> On Aug 24, 2017, at 1:54 PM, Josh Elser <jo...@gmail.com> wrote:
> 
> Sorry I didn't respond sooner (treading water at $dayjob to stay afloat). This is a good conversation that we should have as a community.
> 
> I have to echo Michael's assessment of feeling "uncomfortable" reviewing many contributions due to lack of experience. I recognize that this is a bit of a fallacy, so, sorry I haven't been able to step up more.
> 
> Abstractly, figuring out the right balance for trust in new contributions would help remove such a worry. For example, how much trust do I put in a contribution I might not fully understand if the tests pass?
> 
> If we can get there, at least the only show-stopper is my free time ;)
> 
> On 8/24/17 3:40 PM, Julian Hyde wrote:
>> Thanks for your support, Michael.
>> Not a peep from the rest of you. I suppose there’s no incentive to speak up while the free lunch keeps on coming.
>> What should I do to get some assistance from the many people that benefit from this being a healthy community? Do I have to threaten to go on strike?
>> Julian
>>> On Aug 22, 2017, at 8:01 AM, Michael Mior <mm...@uwaterloo.ca> wrote:
>>> 
>>> I agree this is an important problem to solve. The biggest barrier for me
>>> is that I'm not familiar with most of the Calcite code base and I don't
>>> really have the time to invest right now in order to become familiar to the
>>> point I'd be comfortable reviewing the majority of PRs. I do have
>>> notifications of PRs enabled for the repo and will try to make an effort to
>>> review when I feel qualified.
>>> 
>>> --
>>> Michael Mior
>>> mmior@apache.org <ma...@apache.org> <mailto:mmior@apache.org <ma...@apache.org>>
>>> 
>>> 2017-08-21 15:33 GMT-04:00 Julian Hyde <jhyde@apache.org <ma...@apache.org> <mailto:jhyde@apache.org <ma...@apache.org>>>:
>>> 
>>>> I was on vacation last week. Before I went on vacation I sent an email to
>>>> this list[0] asking the community of committers to stay on top of pull
>>>> requests. My rationale, not explicitly stated in the email but hopefully
>>>> clear to everyone, is that we need to respond to contributors in a timely
>>>> fashion, otherwise they will not join the community. “Timely” means a day
>>>> or two, maximum.
>>>> 
>>>> Since I went on vacation 8 days ago, an existing PR was committed, but 10
>>>> PRs have been created[1], and only one of them received feedback [2].
>>>> (Thanks, Jesus, for the commit and the review.) Several JIRA cases have
>>>> been logged, also.
>>>> 
>>>> I don’t think this is an acceptable amount of feedback to sustain the
>>>> community.
>>>> 
>>>> The Calcite community is successful and growing, but it takes work to keep
>>>> it going. I’m tired of being the main person who does that work. Committers
>>>> and PMC members, you all benefit from the community. You (rightly) expect
>>>> that your own PRs are reviewed and committed in a timely manner. How can I
>>>> encourage you to take a greater share of the work? Do I need to take more
>>>> vacation?
>>>> 
>>>> Julian
>>>> 
>>>> [0] http://mail-archives.apache.org/mod_mbox/calcite-dev/ <http://mail-archives.apache.org/mod_mbox/calcite-dev/>
>>>> 201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D%40apache.org <http://40apache.org/>%3E <
>>>> http://mail-archives.apache.org/mod_mbox/calcite-dev/ <http://mail-archives.apache.org/mod_mbox/calcite-dev/> <http://mail-archives.apache.org/mod_mbox/calcite-dev/ <http://mail-archives.apache.org/mod_mbox/calcite-dev/>>
>>>> 201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D@apache.org <ma...@apache.org> <mailto:3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D@apache.org <ma...@apache.org>>%3E>
>>>> 
>>>> [1] https://github.com/apache/calcite/pulls <https://github.com/apache/calcite/pulls> <https://github.com/apache/calcite/pulls <https://github.com/apache/calcite/pulls>> <https://github.com/apache/ <https://github.com/apache/> <https://github.com/apache/ <https://github.com/apache/>>
>>>> calcite/pulls>
>>>> 
>>>> [2] https://github.com/apache/calcite/pull/523#pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412> <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412>>
>>>> <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412> <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412>>>



Re: [DISCUSS] Reviewing pull requests

Posted by Julian Hyde <jh...@apache.org>.
You never really know whether you have thought of everything. Even if the PR has added tests, and the tests pass, have they tested everything that needs to be tested? If there is an architectural change (and there almost always is some architectural impact, even if it is just doing “more of the same” in an architectural area that is getting a bit creaky) how much burden do we put on the contributor to solve it?

The best you can do is make your best judgment call. I promise never to criticize someone who does what they think best, and I’m sure the other committers feel that way.

That said, if you have carried out an initial review and you think a second opinion would help, just ask for it.

And by the way, as a project we have never decided whether we were commit-then-review (CTR) or review-then-commit (RTC). The policy has been “do what you think best”, and in my opinion that policy is working. If you are a committer and you think your code doesn’t need review, just go ahead and commit it. I often do this, whereas most other committers usually seem to ask for review (this week I have reviewed PRs from Maryann and Minji, who are both committers), but I am not in a privileged position. Just use your discretion, and you won’t be blamed for it.

Julian


> On Aug 24, 2017, at 1:54 PM, Josh Elser <jo...@gmail.com> wrote:
> 
> Sorry I didn't respond sooner (treading water at $dayjob to stay afloat). This is a good conversation that we should have as a community.
> 
> I have to echo Michael's assessment of feeling "uncomfortable" reviewing many contributions due to lack of experience. I recognize that this is a bit of a fallacy, so, sorry I haven't been able to step up more.
> 
> Abstractly, figuring out the right balance for trust in new contributions would help remove such a worry. For example, how much trust do I put in a contribution I might not fully understand if the tests pass?
> 
> If we can get there, at least the only show-stopper is my free time ;)
> 
> On 8/24/17 3:40 PM, Julian Hyde wrote:
>> Thanks for your support, Michael.
>> Not a peep from the rest of you. I suppose there’s no incentive to speak up while the free lunch keeps on coming.
>> What should I do to get some assistance from the many people that benefit from this being a healthy community? Do I have to threaten to go on strike?
>> Julian
>>> On Aug 22, 2017, at 8:01 AM, Michael Mior <mm...@uwaterloo.ca> wrote:
>>> 
>>> I agree this is an important problem to solve. The biggest barrier for me
>>> is that I'm not familiar with most of the Calcite code base and I don't
>>> really have the time to invest right now in order to become familiar to the
>>> point I'd be comfortable reviewing the majority of PRs. I do have
>>> notifications of PRs enabled for the repo and will try to make an effort to
>>> review when I feel qualified.
>>> 
>>> --
>>> Michael Mior
>>> mmior@apache.org <ma...@apache.org> <mailto:mmior@apache.org <ma...@apache.org>>
>>> 
>>> 2017-08-21 15:33 GMT-04:00 Julian Hyde <jhyde@apache.org <ma...@apache.org> <mailto:jhyde@apache.org <ma...@apache.org>>>:
>>> 
>>>> I was on vacation last week. Before I went on vacation I sent an email to
>>>> this list[0] asking the community of committers to stay on top of pull
>>>> requests. My rationale, not explicitly stated in the email but hopefully
>>>> clear to everyone, is that we need to respond to contributors in a timely
>>>> fashion, otherwise they will not join the community. “Timely” means a day
>>>> or two, maximum.
>>>> 
>>>> Since I went on vacation 8 days ago, an existing PR was committed, but 10
>>>> PRs have been created[1], and only one of them received feedback [2].
>>>> (Thanks, Jesus, for the commit and the review.) Several JIRA cases have
>>>> been logged, also.
>>>> 
>>>> I don’t think this is an acceptable amount of feedback to sustain the
>>>> community.
>>>> 
>>>> The Calcite community is successful and growing, but it takes work to keep
>>>> it going. I’m tired of being the main person who does that work. Committers
>>>> and PMC members, you all benefit from the community. You (rightly) expect
>>>> that your own PRs are reviewed and committed in a timely manner. How can I
>>>> encourage you to take a greater share of the work? Do I need to take more
>>>> vacation?
>>>> 
>>>> Julian
>>>> 
>>>> [0] http://mail-archives.apache.org/mod_mbox/calcite-dev/ <http://mail-archives.apache.org/mod_mbox/calcite-dev/>
>>>> 201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D%40apache.org <http://40apache.org/>%3E <
>>>> http://mail-archives.apache.org/mod_mbox/calcite-dev/ <http://mail-archives.apache.org/mod_mbox/calcite-dev/> <http://mail-archives.apache.org/mod_mbox/calcite-dev/ <http://mail-archives.apache.org/mod_mbox/calcite-dev/>>
>>>> 201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D@apache.org <ma...@apache.org> <mailto:3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D@apache.org <ma...@apache.org>>%3E>
>>>> 
>>>> [1] https://github.com/apache/calcite/pulls <https://github.com/apache/calcite/pulls> <https://github.com/apache/calcite/pulls <https://github.com/apache/calcite/pulls>> <https://github.com/apache/ <https://github.com/apache/> <https://github.com/apache/ <https://github.com/apache/>>
>>>> calcite/pulls>
>>>> 
>>>> [2] https://github.com/apache/calcite/pull/523#pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412> <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412>>
>>>> <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412> <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412>>>


Re: [DISCUSS] Reviewing pull requests

Posted by Josh Elser <jo...@gmail.com>.
Sorry I didn't respond sooner (treading water at $dayjob to stay 
afloat). This is a good conversation that we should have as a community.

I have to echo Michael's assessment of feeling "uncomfortable" reviewing 
many contributions due to lack of experience. I recognize that this is a 
bit of a fallacy, so, sorry I haven't been able to step up more.

Abstractly, figuring out the right balance for trust in new 
contributions would help remove such a worry. For example, how much 
trust do I put in a contribution I might not fully understand if the 
tests pass?

If we can get there, at least the only show-stopper is my free time ;)

On 8/24/17 3:40 PM, Julian Hyde wrote:
> Thanks for your support, Michael.
> 
> Not a peep from the rest of you. I suppose there’s no incentive to speak up while the free lunch keeps on coming.
> 
> What should I do to get some assistance from the many people that benefit from this being a healthy community? Do I have to threaten to go on strike?
> 
> Julian
> 
> 
> 
>> On Aug 22, 2017, at 8:01 AM, Michael Mior <mm...@uwaterloo.ca> wrote:
>>
>> I agree this is an important problem to solve. The biggest barrier for me
>> is that I'm not familiar with most of the Calcite code base and I don't
>> really have the time to invest right now in order to become familiar to the
>> point I'd be comfortable reviewing the majority of PRs. I do have
>> notifications of PRs enabled for the repo and will try to make an effort to
>> review when I feel qualified.
>>
>> --
>> Michael Mior
>> mmior@apache.org <ma...@apache.org>
>>
>> 2017-08-21 15:33 GMT-04:00 Julian Hyde <jhyde@apache.org <ma...@apache.org>>:
>>
>>> I was on vacation last week. Before I went on vacation I sent an email to
>>> this list[0] asking the community of committers to stay on top of pull
>>> requests. My rationale, not explicitly stated in the email but hopefully
>>> clear to everyone, is that we need to respond to contributors in a timely
>>> fashion, otherwise they will not join the community. “Timely” means a day
>>> or two, maximum.
>>>
>>> Since I went on vacation 8 days ago, an existing PR was committed, but 10
>>> PRs have been created[1], and only one of them received feedback [2].
>>> (Thanks, Jesus, for the commit and the review.) Several JIRA cases have
>>> been logged, also.
>>>
>>> I don’t think this is an acceptable amount of feedback to sustain the
>>> community.
>>>
>>> The Calcite community is successful and growing, but it takes work to keep
>>> it going. I’m tired of being the main person who does that work. Committers
>>> and PMC members, you all benefit from the community. You (rightly) expect
>>> that your own PRs are reviewed and committed in a timely manner. How can I
>>> encourage you to take a greater share of the work? Do I need to take more
>>> vacation?
>>>
>>> Julian
>>>
>>> [0] http://mail-archives.apache.org/mod_mbox/calcite-dev/
>>> 201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D%40apache.org%3E <
>>> http://mail-archives.apache.org/mod_mbox/calcite-dev/ <http://mail-archives.apache.org/mod_mbox/calcite-dev/>
>>> 201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D@apache.org <ma...@apache.org>%3E>
>>>
>>> [1] https://github.com/apache/calcite/pulls <https://github.com/apache/calcite/pulls> <https://github.com/apache/ <https://github.com/apache/>
>>> calcite/pulls>
>>>
>>> [2] https://github.com/apache/calcite/pull/523#pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412>
>>> <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412>>
> 
> 

Re: [DISCUSS] Reviewing pull requests

Posted by Julian Hyde <jh...@apache.org>.
Thanks for your support, Michael.

Not a peep from the rest of you. I suppose there’s no incentive to speak up while the free lunch keeps on coming.

What should I do to get some assistance from the many people that benefit from this being a healthy community? Do I have to threaten to go on strike?

Julian



> On Aug 22, 2017, at 8:01 AM, Michael Mior <mm...@uwaterloo.ca> wrote:
> 
> I agree this is an important problem to solve. The biggest barrier for me
> is that I'm not familiar with most of the Calcite code base and I don't
> really have the time to invest right now in order to become familiar to the
> point I'd be comfortable reviewing the majority of PRs. I do have
> notifications of PRs enabled for the repo and will try to make an effort to
> review when I feel qualified.
> 
> --
> Michael Mior
> mmior@apache.org <ma...@apache.org>
> 
> 2017-08-21 15:33 GMT-04:00 Julian Hyde <jhyde@apache.org <ma...@apache.org>>:
> 
>> I was on vacation last week. Before I went on vacation I sent an email to
>> this list[0] asking the community of committers to stay on top of pull
>> requests. My rationale, not explicitly stated in the email but hopefully
>> clear to everyone, is that we need to respond to contributors in a timely
>> fashion, otherwise they will not join the community. “Timely” means a day
>> or two, maximum.
>> 
>> Since I went on vacation 8 days ago, an existing PR was committed, but 10
>> PRs have been created[1], and only one of them received feedback [2].
>> (Thanks, Jesus, for the commit and the review.) Several JIRA cases have
>> been logged, also.
>> 
>> I don’t think this is an acceptable amount of feedback to sustain the
>> community.
>> 
>> The Calcite community is successful and growing, but it takes work to keep
>> it going. I’m tired of being the main person who does that work. Committers
>> and PMC members, you all benefit from the community. You (rightly) expect
>> that your own PRs are reviewed and committed in a timely manner. How can I
>> encourage you to take a greater share of the work? Do I need to take more
>> vacation?
>> 
>> Julian
>> 
>> [0] http://mail-archives.apache.org/mod_mbox/calcite-dev/
>> 201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D%40apache.org%3E <
>> http://mail-archives.apache.org/mod_mbox/calcite-dev/ <http://mail-archives.apache.org/mod_mbox/calcite-dev/>
>> 201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D@apache.org <ma...@apache.org>%3E>
>> 
>> [1] https://github.com/apache/calcite/pulls <https://github.com/apache/calcite/pulls> <https://github.com/apache/ <https://github.com/apache/>
>> calcite/pulls>
>> 
>> [2] https://github.com/apache/calcite/pull/523#pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412>
>> <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412 <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412>>


Re: [DISCUSS] Reviewing pull requests

Posted by Michael Mior <mm...@uwaterloo.ca>.
I agree this is an important problem to solve. The biggest barrier for me
is that I'm not familiar with most of the Calcite code base and I don't
really have the time to invest right now in order to become familiar to the
point I'd be comfortable reviewing the majority of PRs. I do have
notifications of PRs enabled for the repo and will try to make an effort to
review when I feel qualified.

--
Michael Mior
mmior@apache.org

2017-08-21 15:33 GMT-04:00 Julian Hyde <jh...@apache.org>:

> I was on vacation last week. Before I went on vacation I sent an email to
> this list[0] asking the community of committers to stay on top of pull
> requests. My rationale, not explicitly stated in the email but hopefully
> clear to everyone, is that we need to respond to contributors in a timely
> fashion, otherwise they will not join the community. “Timely” means a day
> or two, maximum.
>
> Since I went on vacation 8 days ago, an existing PR was committed, but 10
> PRs have been created[1], and only one of them received feedback [2].
> (Thanks, Jesus, for the commit and the review.) Several JIRA cases have
> been logged, also.
>
> I don’t think this is an acceptable amount of feedback to sustain the
> community.
>
> The Calcite community is successful and growing, but it takes work to keep
> it going. I’m tired of being the main person who does that work. Committers
> and PMC members, you all benefit from the community. You (rightly) expect
> that your own PRs are reviewed and committed in a timely manner. How can I
> encourage you to take a greater share of the work? Do I need to take more
> vacation?
>
> Julian
>
> [0] http://mail-archives.apache.org/mod_mbox/calcite-dev/
> 201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D%40apache.org%3E <
> http://mail-archives.apache.org/mod_mbox/calcite-dev/
> 201708.mbox/%3CFF047CF1-7B7D-4F4B-BB05-764BB7F9E70D@apache.org%3E>
>
> [1] https://github.com/apache/calcite/pulls <https://github.com/apache/
> calcite/pulls>
>
> [2] https://github.com/apache/calcite/pull/523#pullrequestreview-57275412
> <https://github.com/apache/calcite/pull/523#pullrequestreview-57275412>