You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Paul Rogers <pr...@mapr.com> on 2017/10/19 00:54:52 UTC
Re: Excessive review comments
With all due respect, I did start a review. Is something broken?
- Paul
> On Oct 18, 2017, at 3:36 PM, julianhyde <gi...@git.apache.org> wrote:
>
> Github user julianhyde commented on a diff in the pull request:
>
> https://github.com/apache/drill/pull/984#discussion_r145561518
>
> --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
> @@ -584,11 +492,14 @@ public static void defineWorkspace(Drillbit drillbit, String pluginName,
> public static final String EXPLAIN_PLAN_TEXT = "text";
> public static final String EXPLAIN_PLAN_JSON = "json";
>
> - public static FixtureBuilder builder() {
> - FixtureBuilder builder = new FixtureBuilder()
> + public static FixtureBuilder builder(DirTestWatcher dirTestWatcher) {
> --- End diff --
>
> Jeez Paul, please start a review rather than making single review comments. I just got 39 emails from you, and so did everyone else on dev@drill.
>
>
> ---
Re: Excessive review comments
Posted by Parth Chandra <pa...@apache.org>.
I would prefer a solution that does not require subscribing to individual
PRs or JIRAs. How would one know what is interesting if one doesn't get
notified?
On Thu, Oct 19, 2017 at 10:13 AM, Paul Rogers <pr...@mapr.com> wrote:
> So, rather than copying all messages to dev, simply ask interested folks
> subscribe to the PRs of interest. (Or, subscribe to the corresponding JIRA
> tickets, since JIRA echos all comments in another spray of e-mail…)
>
> This way, semi-active folks on the dev list can see substantive
> discussions without being bombarded with day-to-day minutia.
>
> Thanks,
>
> - Paul
>
> > On Oct 19, 2017, at 10:07 AM, Parth Chandra <pa...@apache.org> wrote:
> >
> > I generally keep track of dev activity only through the emails sent. I
> > don't mind getting duplicate emails either - it's not too hard to write a
> > filter to take out the stuff you're not interested in.
> >
> > Interestingly, I've never noticed whether 'start a review' sends one
> email
> > or many; mostly because gmail does a nice job of grouping the emails
> > together. I have seen a single email with many comments being sent out so
> > the feature did work as advertised at one point.
> >
> > That said, if there is a way I can stay updated via email (repo watching,
> > jira updates, etc.) then I am fine with turning the feature off.
> >
> >
> >
> > On Thu, Oct 19, 2017 at 9:51 AM, Timothy Farkas <tf...@mapr.com>
> wrote:
> >
> >> +1 for turning off the feature. If someone really needs to be emailed
> with
> >> comment updates they can become a watcher of the repo on Github.
> >>
> >> ________________________________
> >> From: Paul Rogers <pr...@mapr.com>
> >> Sent: Thursday, October 19, 2017 9:43:26 AM
> >> To: dev@drill.apache.org
> >> Subject: Re: Excessive review comments
> >>
> >> Can we simply turn off the feature? I never, ever read the e-mails
> coming
> >> from this source; I always follow the link back to the PR. Can we
> reduce it
> >> to “Hey, just wanted to let you know that a new comment was posted.
> Click
> >> _here_ to read it.”
> >>
> >> The only other solution is to give few review comments; not sure if we
> >> want to go that route...
> >>
> >> - Paul
> >>
> >>> On Oct 19, 2017, at 8:35 AM, Arina Yelchiyeva <
> >> arina.yelchiyeva@gmail.com> wrote:
> >>>
> >>> Agree, I am not sure I saw this feature working as well.
> >>> All it did it was sending all the emails at once, rather in the process
> >> of
> >>> comments emergence.
> >>>
> >>> Kind regards
> >>> Arina
> >>>
> >>> On Thu, Oct 19, 2017 at 6:27 PM, Julian Hyde <jh...@apache.org> wrote:
> >>>
> >>>> I don’t know whether anything is broken. I believed that the GitHub
> >> “start
> >>>> a review” feature would cause all review comments to be sent in a
> single
> >>>> email. But now I think of it, I’m not sure I ever saw it working. I
> >> wonder
> >>>> whether Github-ASF integration is at fault.
> >>>>
> >>>> Whatever the reasons for it, 39 emails to dev list is quite a blast.
> >>>> People tend to unsubscribe from lists if the volume is too high.
> >>>>
> >>>> Julian
> >>>>
> >>>>
> >>>>> On Oct 18, 2017, at 5:54 PM, Paul Rogers <pr...@mapr.com> wrote:
> >>>>>
> >>>>> With all due respect, I did start a review. Is something broken?
> >>>>>
> >>>>> - Paul
> >>>>>
> >>>>>> On Oct 18, 2017, at 3:36 PM, julianhyde <gi...@git.apache.org> wrote:
> >>>>>>
> >>>>>> Github user julianhyde commented on a diff in the pull request:
> >>>>>>
> >>>>>> https://github.com/apache/drill/pull/984#discussion_r145561518
> >>>>>>
> >>>>>> --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/
> >> ClusterFixture.java
> >>>> ---
> >>>>>> @@ -584,11 +492,14 @@ public static void defineWorkspace(Drillbit
> >>>> drillbit, String pluginName,
> >>>>>> public static final String EXPLAIN_PLAN_TEXT = "text";
> >>>>>> public static final String EXPLAIN_PLAN_JSON = "json";
> >>>>>>
> >>>>>> - public static FixtureBuilder builder() {
> >>>>>> - FixtureBuilder builder = new FixtureBuilder()
> >>>>>> + public static FixtureBuilder builder(DirTestWatcher
> >>>> dirTestWatcher) {
> >>>>>> --- End diff --
> >>>>>>
> >>>>>> Jeez Paul, please start a review rather than making single review
> >>>> comments. I just got 39 emails from you, and so did everyone else on
> >>>> dev@drill.
> >>>>>>
> >>>>>>
> >>>>>> ---
> >>>>>
> >>>>
> >>>>
> >>
> >>
>
>
Re: Excessive review comments
Posted by Arina Yelchiyeva <ar...@gmail.com>.
Ideally, there should be an option if you want to receive all notifications
from github or not. All these notifications don't really bother me. As
Parth mentioned this is the way to track dev activity.
Subscribing to the pull request maybe not always be the best solution,
since sometimes the comments in pull request is the point of interest and
gets you engaged into the discussion.
Regarding the code review comments, I think we should not point to the code
reviewers how they should write comments until they provide good code
review and raise good questions.
Especially taking into account limited number of volunteers to do the code
review...
Kind regards
Arina
On Thu, Oct 19, 2017 at 9:55 PM, Aman Sinha <am...@apache.org> wrote:
> Going back to one comment by Paul : “The only other solution is to give few
> review comments; not sure if we want to go that route...”
> IMO the individual code review comments should be concise such that the
> main idea is expressed. This is more palatable for the original developer
> and he/she can act on it. If there are several comments that are somewhat
> lengthier, it amounts to a design discussion. These would be better
> combined and could either be (a) added to the JIRA or (b) added to the
> ‘conversation’ section of the PR to minimize notifications.
>
> -Aman
>
> On Thu, Oct 19, 2017 at 10:59 AM, Padma Penumarthy <pp...@mapr.com>
> wrote:
>
> > To subscribe to PRs and JIRAs of interest, we need to know about them
> > first.
> > Is it possible to get notified when a new PR or JIRA is created and not
> for
> > further updates unless you subscribe to them ?
> >
> > Thanks
> > Padma
> >
> >
> > > On Oct 19, 2017, at 10:13 AM, Paul Rogers <pr...@mapr.com> wrote:
> > >
> > > So, rather than copying all messages to dev, simply ask interested
> folks
> > subscribe to the PRs of interest. (Or, subscribe to the corresponding
> JIRA
> > tickets, since JIRA echos all comments in another spray of e-mail…)
> > >
> > > This way, semi-active folks on the dev list can see substantive
> > discussions without being bombarded with day-to-day minutia.
> > >
> > > Thanks,
> > >
> > > - Paul
> > >
> > >> On Oct 19, 2017, at 10:07 AM, Parth Chandra <pa...@apache.org>
> wrote:
> > >>
> > >> I generally keep track of dev activity only through the emails sent. I
> > >> don't mind getting duplicate emails either - it's not too hard to
> write
> > a
> > >> filter to take out the stuff you're not interested in.
> > >>
> > >> Interestingly, I've never noticed whether 'start a review' sends one
> > email
> > >> or many; mostly because gmail does a nice job of grouping the emails
> > >> together. I have seen a single email with many comments being sent out
> > so
> > >> the feature did work as advertised at one point.
> > >>
> > >> That said, if there is a way I can stay updated via email (repo
> > watching,
> > >> jira updates, etc.) then I am fine with turning the feature off.
> > >>
> > >>
> > >>
> > >> On Thu, Oct 19, 2017 at 9:51 AM, Timothy Farkas <tf...@mapr.com>
> > wrote:
> > >>
> > >>> +1 for turning off the feature. If someone really needs to be emailed
> > with
> > >>> comment updates they can become a watcher of the repo on Github.
> > >>>
> > >>> ________________________________
> > >>> From: Paul Rogers <pr...@mapr.com>
> > >>> Sent: Thursday, October 19, 2017 9:43:26 AM
> > >>> To: dev@drill.apache.org
> > >>> Subject: Re: Excessive review comments
> > >>>
> > >>> Can we simply turn off the feature? I never, ever read the e-mails
> > coming
> > >>> from this source; I always follow the link back to the PR. Can we
> > reduce it
> > >>> to “Hey, just wanted to let you know that a new comment was posted.
> > Click
> > >>> _here_ to read it.”
> > >>>
> > >>> The only other solution is to give few review comments; not sure if
> we
> > >>> want to go that route...
> > >>>
> > >>> - Paul
> > >>>
> > >>>> On Oct 19, 2017, at 8:35 AM, Arina Yelchiyeva <
> > >>> arina.yelchiyeva@gmail.com> wrote:
> > >>>>
> > >>>> Agree, I am not sure I saw this feature working as well.
> > >>>> All it did it was sending all the emails at once, rather in the
> > process
> > >>> of
> > >>>> comments emergence.
> > >>>>
> > >>>> Kind regards
> > >>>> Arina
> > >>>>
> > >>>> On Thu, Oct 19, 2017 at 6:27 PM, Julian Hyde <jh...@apache.org>
> > wrote:
> > >>>>
> > >>>>> I don’t know whether anything is broken. I believed that the GitHub
> > >>> “start
> > >>>>> a review” feature would cause all review comments to be sent in a
> > single
> > >>>>> email. But now I think of it, I’m not sure I ever saw it working. I
> > >>> wonder
> > >>>>> whether Github-ASF integration is at fault.
> > >>>>>
> > >>>>> Whatever the reasons for it, 39 emails to dev list is quite a
> blast.
> > >>>>> People tend to unsubscribe from lists if the volume is too high.
> > >>>>>
> > >>>>> Julian
> > >>>>>
> > >>>>>
> > >>>>>> On Oct 18, 2017, at 5:54 PM, Paul Rogers <pr...@mapr.com>
> wrote:
> > >>>>>>
> > >>>>>> With all due respect, I did start a review. Is something broken?
> > >>>>>>
> > >>>>>> - Paul
> > >>>>>>
> > >>>>>>> On Oct 18, 2017, at 3:36 PM, julianhyde <gi...@git.apache.org>
> > wrote:
> > >>>>>>>
> > >>>>>>> Github user julianhyde commented on a diff in the pull request:
> > >>>>>>>
> > >>>>>>> https://github.com/apache/drill/pull/984#discussion_r145561518
> > >>>>>>>
> > >>>>>>> --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/
> > >>> ClusterFixture.java
> > >>>>> ---
> > >>>>>>> @@ -584,11 +492,14 @@ public static void defineWorkspace(Drillbit
> > >>>>> drillbit, String pluginName,
> > >>>>>>> public static final String EXPLAIN_PLAN_TEXT = "text";
> > >>>>>>> public static final String EXPLAIN_PLAN_JSON = "json";
> > >>>>>>>
> > >>>>>>> - public static FixtureBuilder builder() {
> > >>>>>>> - FixtureBuilder builder = new FixtureBuilder()
> > >>>>>>> + public static FixtureBuilder builder(DirTestWatcher
> > >>>>> dirTestWatcher) {
> > >>>>>>> --- End diff --
> > >>>>>>>
> > >>>>>>> Jeez Paul, please start a review rather than making single review
> > >>>>> comments. I just got 39 emails from you, and so did everyone else
> on
> > >>>>> dev@drill.
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> ---
> > >>>>>>
> > >>>>>
> > >>>>>
> > >>>
> > >>>
> > >
> >
> >
>
Re: Excessive review comments
Posted by Aman Sinha <am...@apache.org>.
Going back to one comment by Paul : “The only other solution is to give few
review comments; not sure if we want to go that route...”
IMO the individual code review comments should be concise such that the
main idea is expressed. This is more palatable for the original developer
and he/she can act on it. If there are several comments that are somewhat
lengthier, it amounts to a design discussion. These would be better
combined and could either be (a) added to the JIRA or (b) added to the
‘conversation’ section of the PR to minimize notifications.
-Aman
On Thu, Oct 19, 2017 at 10:59 AM, Padma Penumarthy <pp...@mapr.com>
wrote:
> To subscribe to PRs and JIRAs of interest, we need to know about them
> first.
> Is it possible to get notified when a new PR or JIRA is created and not for
> further updates unless you subscribe to them ?
>
> Thanks
> Padma
>
>
> > On Oct 19, 2017, at 10:13 AM, Paul Rogers <pr...@mapr.com> wrote:
> >
> > So, rather than copying all messages to dev, simply ask interested folks
> subscribe to the PRs of interest. (Or, subscribe to the corresponding JIRA
> tickets, since JIRA echos all comments in another spray of e-mail…)
> >
> > This way, semi-active folks on the dev list can see substantive
> discussions without being bombarded with day-to-day minutia.
> >
> > Thanks,
> >
> > - Paul
> >
> >> On Oct 19, 2017, at 10:07 AM, Parth Chandra <pa...@apache.org> wrote:
> >>
> >> I generally keep track of dev activity only through the emails sent. I
> >> don't mind getting duplicate emails either - it's not too hard to write
> a
> >> filter to take out the stuff you're not interested in.
> >>
> >> Interestingly, I've never noticed whether 'start a review' sends one
> email
> >> or many; mostly because gmail does a nice job of grouping the emails
> >> together. I have seen a single email with many comments being sent out
> so
> >> the feature did work as advertised at one point.
> >>
> >> That said, if there is a way I can stay updated via email (repo
> watching,
> >> jira updates, etc.) then I am fine with turning the feature off.
> >>
> >>
> >>
> >> On Thu, Oct 19, 2017 at 9:51 AM, Timothy Farkas <tf...@mapr.com>
> wrote:
> >>
> >>> +1 for turning off the feature. If someone really needs to be emailed
> with
> >>> comment updates they can become a watcher of the repo on Github.
> >>>
> >>> ________________________________
> >>> From: Paul Rogers <pr...@mapr.com>
> >>> Sent: Thursday, October 19, 2017 9:43:26 AM
> >>> To: dev@drill.apache.org
> >>> Subject: Re: Excessive review comments
> >>>
> >>> Can we simply turn off the feature? I never, ever read the e-mails
> coming
> >>> from this source; I always follow the link back to the PR. Can we
> reduce it
> >>> to “Hey, just wanted to let you know that a new comment was posted.
> Click
> >>> _here_ to read it.”
> >>>
> >>> The only other solution is to give few review comments; not sure if we
> >>> want to go that route...
> >>>
> >>> - Paul
> >>>
> >>>> On Oct 19, 2017, at 8:35 AM, Arina Yelchiyeva <
> >>> arina.yelchiyeva@gmail.com> wrote:
> >>>>
> >>>> Agree, I am not sure I saw this feature working as well.
> >>>> All it did it was sending all the emails at once, rather in the
> process
> >>> of
> >>>> comments emergence.
> >>>>
> >>>> Kind regards
> >>>> Arina
> >>>>
> >>>> On Thu, Oct 19, 2017 at 6:27 PM, Julian Hyde <jh...@apache.org>
> wrote:
> >>>>
> >>>>> I don’t know whether anything is broken. I believed that the GitHub
> >>> “start
> >>>>> a review” feature would cause all review comments to be sent in a
> single
> >>>>> email. But now I think of it, I’m not sure I ever saw it working. I
> >>> wonder
> >>>>> whether Github-ASF integration is at fault.
> >>>>>
> >>>>> Whatever the reasons for it, 39 emails to dev list is quite a blast.
> >>>>> People tend to unsubscribe from lists if the volume is too high.
> >>>>>
> >>>>> Julian
> >>>>>
> >>>>>
> >>>>>> On Oct 18, 2017, at 5:54 PM, Paul Rogers <pr...@mapr.com> wrote:
> >>>>>>
> >>>>>> With all due respect, I did start a review. Is something broken?
> >>>>>>
> >>>>>> - Paul
> >>>>>>
> >>>>>>> On Oct 18, 2017, at 3:36 PM, julianhyde <gi...@git.apache.org>
> wrote:
> >>>>>>>
> >>>>>>> Github user julianhyde commented on a diff in the pull request:
> >>>>>>>
> >>>>>>> https://github.com/apache/drill/pull/984#discussion_r145561518
> >>>>>>>
> >>>>>>> --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/
> >>> ClusterFixture.java
> >>>>> ---
> >>>>>>> @@ -584,11 +492,14 @@ public static void defineWorkspace(Drillbit
> >>>>> drillbit, String pluginName,
> >>>>>>> public static final String EXPLAIN_PLAN_TEXT = "text";
> >>>>>>> public static final String EXPLAIN_PLAN_JSON = "json";
> >>>>>>>
> >>>>>>> - public static FixtureBuilder builder() {
> >>>>>>> - FixtureBuilder builder = new FixtureBuilder()
> >>>>>>> + public static FixtureBuilder builder(DirTestWatcher
> >>>>> dirTestWatcher) {
> >>>>>>> --- End diff --
> >>>>>>>
> >>>>>>> Jeez Paul, please start a review rather than making single review
> >>>>> comments. I just got 39 emails from you, and so did everyone else on
> >>>>> dev@drill.
> >>>>>>>
> >>>>>>>
> >>>>>>> ---
> >>>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >
>
>
Re: Excessive review comments
Posted by Padma Penumarthy <pp...@mapr.com>.
To subscribe to PRs and JIRAs of interest, we need to know about them first.
Is it possible to get notified when a new PR or JIRA is created and not for
further updates unless you subscribe to them ?
Thanks
Padma
> On Oct 19, 2017, at 10:13 AM, Paul Rogers <pr...@mapr.com> wrote:
>
> So, rather than copying all messages to dev, simply ask interested folks subscribe to the PRs of interest. (Or, subscribe to the corresponding JIRA tickets, since JIRA echos all comments in another spray of e-mail…)
>
> This way, semi-active folks on the dev list can see substantive discussions without being bombarded with day-to-day minutia.
>
> Thanks,
>
> - Paul
>
>> On Oct 19, 2017, at 10:07 AM, Parth Chandra <pa...@apache.org> wrote:
>>
>> I generally keep track of dev activity only through the emails sent. I
>> don't mind getting duplicate emails either - it's not too hard to write a
>> filter to take out the stuff you're not interested in.
>>
>> Interestingly, I've never noticed whether 'start a review' sends one email
>> or many; mostly because gmail does a nice job of grouping the emails
>> together. I have seen a single email with many comments being sent out so
>> the feature did work as advertised at one point.
>>
>> That said, if there is a way I can stay updated via email (repo watching,
>> jira updates, etc.) then I am fine with turning the feature off.
>>
>>
>>
>> On Thu, Oct 19, 2017 at 9:51 AM, Timothy Farkas <tf...@mapr.com> wrote:
>>
>>> +1 for turning off the feature. If someone really needs to be emailed with
>>> comment updates they can become a watcher of the repo on Github.
>>>
>>> ________________________________
>>> From: Paul Rogers <pr...@mapr.com>
>>> Sent: Thursday, October 19, 2017 9:43:26 AM
>>> To: dev@drill.apache.org
>>> Subject: Re: Excessive review comments
>>>
>>> Can we simply turn off the feature? I never, ever read the e-mails coming
>>> from this source; I always follow the link back to the PR. Can we reduce it
>>> to “Hey, just wanted to let you know that a new comment was posted. Click
>>> _here_ to read it.”
>>>
>>> The only other solution is to give few review comments; not sure if we
>>> want to go that route...
>>>
>>> - Paul
>>>
>>>> On Oct 19, 2017, at 8:35 AM, Arina Yelchiyeva <
>>> arina.yelchiyeva@gmail.com> wrote:
>>>>
>>>> Agree, I am not sure I saw this feature working as well.
>>>> All it did it was sending all the emails at once, rather in the process
>>> of
>>>> comments emergence.
>>>>
>>>> Kind regards
>>>> Arina
>>>>
>>>> On Thu, Oct 19, 2017 at 6:27 PM, Julian Hyde <jh...@apache.org> wrote:
>>>>
>>>>> I don’t know whether anything is broken. I believed that the GitHub
>>> “start
>>>>> a review” feature would cause all review comments to be sent in a single
>>>>> email. But now I think of it, I’m not sure I ever saw it working. I
>>> wonder
>>>>> whether Github-ASF integration is at fault.
>>>>>
>>>>> Whatever the reasons for it, 39 emails to dev list is quite a blast.
>>>>> People tend to unsubscribe from lists if the volume is too high.
>>>>>
>>>>> Julian
>>>>>
>>>>>
>>>>>> On Oct 18, 2017, at 5:54 PM, Paul Rogers <pr...@mapr.com> wrote:
>>>>>>
>>>>>> With all due respect, I did start a review. Is something broken?
>>>>>>
>>>>>> - Paul
>>>>>>
>>>>>>> On Oct 18, 2017, at 3:36 PM, julianhyde <gi...@git.apache.org> wrote:
>>>>>>>
>>>>>>> Github user julianhyde commented on a diff in the pull request:
>>>>>>>
>>>>>>> https://github.com/apache/drill/pull/984#discussion_r145561518
>>>>>>>
>>>>>>> --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/
>>> ClusterFixture.java
>>>>> ---
>>>>>>> @@ -584,11 +492,14 @@ public static void defineWorkspace(Drillbit
>>>>> drillbit, String pluginName,
>>>>>>> public static final String EXPLAIN_PLAN_TEXT = "text";
>>>>>>> public static final String EXPLAIN_PLAN_JSON = "json";
>>>>>>>
>>>>>>> - public static FixtureBuilder builder() {
>>>>>>> - FixtureBuilder builder = new FixtureBuilder()
>>>>>>> + public static FixtureBuilder builder(DirTestWatcher
>>>>> dirTestWatcher) {
>>>>>>> --- End diff --
>>>>>>>
>>>>>>> Jeez Paul, please start a review rather than making single review
>>>>> comments. I just got 39 emails from you, and so did everyone else on
>>>>> dev@drill.
>>>>>>>
>>>>>>>
>>>>>>> ---
>>>>>>
>>>>>
>>>>>
>>>
>>>
>
Re: Excessive review comments
Posted by Paul Rogers <pr...@mapr.com>.
So, rather than copying all messages to dev, simply ask interested folks subscribe to the PRs of interest. (Or, subscribe to the corresponding JIRA tickets, since JIRA echos all comments in another spray of e-mail…)
This way, semi-active folks on the dev list can see substantive discussions without being bombarded with day-to-day minutia.
Thanks,
- Paul
> On Oct 19, 2017, at 10:07 AM, Parth Chandra <pa...@apache.org> wrote:
>
> I generally keep track of dev activity only through the emails sent. I
> don't mind getting duplicate emails either - it's not too hard to write a
> filter to take out the stuff you're not interested in.
>
> Interestingly, I've never noticed whether 'start a review' sends one email
> or many; mostly because gmail does a nice job of grouping the emails
> together. I have seen a single email with many comments being sent out so
> the feature did work as advertised at one point.
>
> That said, if there is a way I can stay updated via email (repo watching,
> jira updates, etc.) then I am fine with turning the feature off.
>
>
>
> On Thu, Oct 19, 2017 at 9:51 AM, Timothy Farkas <tf...@mapr.com> wrote:
>
>> +1 for turning off the feature. If someone really needs to be emailed with
>> comment updates they can become a watcher of the repo on Github.
>>
>> ________________________________
>> From: Paul Rogers <pr...@mapr.com>
>> Sent: Thursday, October 19, 2017 9:43:26 AM
>> To: dev@drill.apache.org
>> Subject: Re: Excessive review comments
>>
>> Can we simply turn off the feature? I never, ever read the e-mails coming
>> from this source; I always follow the link back to the PR. Can we reduce it
>> to “Hey, just wanted to let you know that a new comment was posted. Click
>> _here_ to read it.”
>>
>> The only other solution is to give few review comments; not sure if we
>> want to go that route...
>>
>> - Paul
>>
>>> On Oct 19, 2017, at 8:35 AM, Arina Yelchiyeva <
>> arina.yelchiyeva@gmail.com> wrote:
>>>
>>> Agree, I am not sure I saw this feature working as well.
>>> All it did it was sending all the emails at once, rather in the process
>> of
>>> comments emergence.
>>>
>>> Kind regards
>>> Arina
>>>
>>> On Thu, Oct 19, 2017 at 6:27 PM, Julian Hyde <jh...@apache.org> wrote:
>>>
>>>> I don’t know whether anything is broken. I believed that the GitHub
>> “start
>>>> a review” feature would cause all review comments to be sent in a single
>>>> email. But now I think of it, I’m not sure I ever saw it working. I
>> wonder
>>>> whether Github-ASF integration is at fault.
>>>>
>>>> Whatever the reasons for it, 39 emails to dev list is quite a blast.
>>>> People tend to unsubscribe from lists if the volume is too high.
>>>>
>>>> Julian
>>>>
>>>>
>>>>> On Oct 18, 2017, at 5:54 PM, Paul Rogers <pr...@mapr.com> wrote:
>>>>>
>>>>> With all due respect, I did start a review. Is something broken?
>>>>>
>>>>> - Paul
>>>>>
>>>>>> On Oct 18, 2017, at 3:36 PM, julianhyde <gi...@git.apache.org> wrote:
>>>>>>
>>>>>> Github user julianhyde commented on a diff in the pull request:
>>>>>>
>>>>>> https://github.com/apache/drill/pull/984#discussion_r145561518
>>>>>>
>>>>>> --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/
>> ClusterFixture.java
>>>> ---
>>>>>> @@ -584,11 +492,14 @@ public static void defineWorkspace(Drillbit
>>>> drillbit, String pluginName,
>>>>>> public static final String EXPLAIN_PLAN_TEXT = "text";
>>>>>> public static final String EXPLAIN_PLAN_JSON = "json";
>>>>>>
>>>>>> - public static FixtureBuilder builder() {
>>>>>> - FixtureBuilder builder = new FixtureBuilder()
>>>>>> + public static FixtureBuilder builder(DirTestWatcher
>>>> dirTestWatcher) {
>>>>>> --- End diff --
>>>>>>
>>>>>> Jeez Paul, please start a review rather than making single review
>>>> comments. I just got 39 emails from you, and so did everyone else on
>>>> dev@drill.
>>>>>>
>>>>>>
>>>>>> ---
>>>>>
>>>>
>>>>
>>
>>
Re: Excessive review comments
Posted by Parth Chandra <pa...@apache.org>.
I generally keep track of dev activity only through the emails sent. I
don't mind getting duplicate emails either - it's not too hard to write a
filter to take out the stuff you're not interested in.
Interestingly, I've never noticed whether 'start a review' sends one email
or many; mostly because gmail does a nice job of grouping the emails
together. I have seen a single email with many comments being sent out so
the feature did work as advertised at one point.
That said, if there is a way I can stay updated via email (repo watching,
jira updates, etc.) then I am fine with turning the feature off.
On Thu, Oct 19, 2017 at 9:51 AM, Timothy Farkas <tf...@mapr.com> wrote:
> +1 for turning off the feature. If someone really needs to be emailed with
> comment updates they can become a watcher of the repo on Github.
>
> ________________________________
> From: Paul Rogers <pr...@mapr.com>
> Sent: Thursday, October 19, 2017 9:43:26 AM
> To: dev@drill.apache.org
> Subject: Re: Excessive review comments
>
> Can we simply turn off the feature? I never, ever read the e-mails coming
> from this source; I always follow the link back to the PR. Can we reduce it
> to “Hey, just wanted to let you know that a new comment was posted. Click
> _here_ to read it.”
>
> The only other solution is to give few review comments; not sure if we
> want to go that route...
>
> - Paul
>
> > On Oct 19, 2017, at 8:35 AM, Arina Yelchiyeva <
> arina.yelchiyeva@gmail.com> wrote:
> >
> > Agree, I am not sure I saw this feature working as well.
> > All it did it was sending all the emails at once, rather in the process
> of
> > comments emergence.
> >
> > Kind regards
> > Arina
> >
> > On Thu, Oct 19, 2017 at 6:27 PM, Julian Hyde <jh...@apache.org> wrote:
> >
> >> I don’t know whether anything is broken. I believed that the GitHub
> “start
> >> a review” feature would cause all review comments to be sent in a single
> >> email. But now I think of it, I’m not sure I ever saw it working. I
> wonder
> >> whether Github-ASF integration is at fault.
> >>
> >> Whatever the reasons for it, 39 emails to dev list is quite a blast.
> >> People tend to unsubscribe from lists if the volume is too high.
> >>
> >> Julian
> >>
> >>
> >>> On Oct 18, 2017, at 5:54 PM, Paul Rogers <pr...@mapr.com> wrote:
> >>>
> >>> With all due respect, I did start a review. Is something broken?
> >>>
> >>> - Paul
> >>>
> >>>> On Oct 18, 2017, at 3:36 PM, julianhyde <gi...@git.apache.org> wrote:
> >>>>
> >>>> Github user julianhyde commented on a diff in the pull request:
> >>>>
> >>>> https://github.com/apache/drill/pull/984#discussion_r145561518
> >>>>
> >>>> --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/
> ClusterFixture.java
> >> ---
> >>>> @@ -584,11 +492,14 @@ public static void defineWorkspace(Drillbit
> >> drillbit, String pluginName,
> >>>> public static final String EXPLAIN_PLAN_TEXT = "text";
> >>>> public static final String EXPLAIN_PLAN_JSON = "json";
> >>>>
> >>>> - public static FixtureBuilder builder() {
> >>>> - FixtureBuilder builder = new FixtureBuilder()
> >>>> + public static FixtureBuilder builder(DirTestWatcher
> >> dirTestWatcher) {
> >>>> --- End diff --
> >>>>
> >>>> Jeez Paul, please start a review rather than making single review
> >> comments. I just got 39 emails from you, and so did everyone else on
> >> dev@drill.
> >>>>
> >>>>
> >>>> ---
> >>>
> >>
> >>
>
>
Re: Excessive review comments
Posted by Timothy Farkas <tf...@mapr.com>.
+1 for turning off the feature. If someone really needs to be emailed with comment updates they can become a watcher of the repo on Github.
________________________________
From: Paul Rogers <pr...@mapr.com>
Sent: Thursday, October 19, 2017 9:43:26 AM
To: dev@drill.apache.org
Subject: Re: Excessive review comments
Can we simply turn off the feature? I never, ever read the e-mails coming from this source; I always follow the link back to the PR. Can we reduce it to “Hey, just wanted to let you know that a new comment was posted. Click _here_ to read it.”
The only other solution is to give few review comments; not sure if we want to go that route...
- Paul
> On Oct 19, 2017, at 8:35 AM, Arina Yelchiyeva <ar...@gmail.com> wrote:
>
> Agree, I am not sure I saw this feature working as well.
> All it did it was sending all the emails at once, rather in the process of
> comments emergence.
>
> Kind regards
> Arina
>
> On Thu, Oct 19, 2017 at 6:27 PM, Julian Hyde <jh...@apache.org> wrote:
>
>> I don’t know whether anything is broken. I believed that the GitHub “start
>> a review” feature would cause all review comments to be sent in a single
>> email. But now I think of it, I’m not sure I ever saw it working. I wonder
>> whether Github-ASF integration is at fault.
>>
>> Whatever the reasons for it, 39 emails to dev list is quite a blast.
>> People tend to unsubscribe from lists if the volume is too high.
>>
>> Julian
>>
>>
>>> On Oct 18, 2017, at 5:54 PM, Paul Rogers <pr...@mapr.com> wrote:
>>>
>>> With all due respect, I did start a review. Is something broken?
>>>
>>> - Paul
>>>
>>>> On Oct 18, 2017, at 3:36 PM, julianhyde <gi...@git.apache.org> wrote:
>>>>
>>>> Github user julianhyde commented on a diff in the pull request:
>>>>
>>>> https://github.com/apache/drill/pull/984#discussion_r145561518
>>>>
>>>> --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java
>> ---
>>>> @@ -584,11 +492,14 @@ public static void defineWorkspace(Drillbit
>> drillbit, String pluginName,
>>>> public static final String EXPLAIN_PLAN_TEXT = "text";
>>>> public static final String EXPLAIN_PLAN_JSON = "json";
>>>>
>>>> - public static FixtureBuilder builder() {
>>>> - FixtureBuilder builder = new FixtureBuilder()
>>>> + public static FixtureBuilder builder(DirTestWatcher
>> dirTestWatcher) {
>>>> --- End diff --
>>>>
>>>> Jeez Paul, please start a review rather than making single review
>> comments. I just got 39 emails from you, and so did everyone else on
>> dev@drill.
>>>>
>>>>
>>>> ---
>>>
>>
>>
Re: Excessive review comments
Posted by Paul Rogers <pr...@mapr.com>.
Can we simply turn off the feature? I never, ever read the e-mails coming from this source; I always follow the link back to the PR. Can we reduce it to “Hey, just wanted to let you know that a new comment was posted. Click _here_ to read it.”
The only other solution is to give few review comments; not sure if we want to go that route...
- Paul
> On Oct 19, 2017, at 8:35 AM, Arina Yelchiyeva <ar...@gmail.com> wrote:
>
> Agree, I am not sure I saw this feature working as well.
> All it did it was sending all the emails at once, rather in the process of
> comments emergence.
>
> Kind regards
> Arina
>
> On Thu, Oct 19, 2017 at 6:27 PM, Julian Hyde <jh...@apache.org> wrote:
>
>> I don’t know whether anything is broken. I believed that the GitHub “start
>> a review” feature would cause all review comments to be sent in a single
>> email. But now I think of it, I’m not sure I ever saw it working. I wonder
>> whether Github-ASF integration is at fault.
>>
>> Whatever the reasons for it, 39 emails to dev list is quite a blast.
>> People tend to unsubscribe from lists if the volume is too high.
>>
>> Julian
>>
>>
>>> On Oct 18, 2017, at 5:54 PM, Paul Rogers <pr...@mapr.com> wrote:
>>>
>>> With all due respect, I did start a review. Is something broken?
>>>
>>> - Paul
>>>
>>>> On Oct 18, 2017, at 3:36 PM, julianhyde <gi...@git.apache.org> wrote:
>>>>
>>>> Github user julianhyde commented on a diff in the pull request:
>>>>
>>>> https://github.com/apache/drill/pull/984#discussion_r145561518
>>>>
>>>> --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java
>> ---
>>>> @@ -584,11 +492,14 @@ public static void defineWorkspace(Drillbit
>> drillbit, String pluginName,
>>>> public static final String EXPLAIN_PLAN_TEXT = "text";
>>>> public static final String EXPLAIN_PLAN_JSON = "json";
>>>>
>>>> - public static FixtureBuilder builder() {
>>>> - FixtureBuilder builder = new FixtureBuilder()
>>>> + public static FixtureBuilder builder(DirTestWatcher
>> dirTestWatcher) {
>>>> --- End diff --
>>>>
>>>> Jeez Paul, please start a review rather than making single review
>> comments. I just got 39 emails from you, and so did everyone else on
>> dev@drill.
>>>>
>>>>
>>>> ---
>>>
>>
>>
Re: Excessive review comments
Posted by Arina Yelchiyeva <ar...@gmail.com>.
Agree, I am not sure I saw this feature working as well.
All it did it was sending all the emails at once, rather in the process of
comments emergence.
Kind regards
Arina
On Thu, Oct 19, 2017 at 6:27 PM, Julian Hyde <jh...@apache.org> wrote:
> I don’t know whether anything is broken. I believed that the GitHub “start
> a review” feature would cause all review comments to be sent in a single
> email. But now I think of it, I’m not sure I ever saw it working. I wonder
> whether Github-ASF integration is at fault.
>
> Whatever the reasons for it, 39 emails to dev list is quite a blast.
> People tend to unsubscribe from lists if the volume is too high.
>
> Julian
>
>
> > On Oct 18, 2017, at 5:54 PM, Paul Rogers <pr...@mapr.com> wrote:
> >
> > With all due respect, I did start a review. Is something broken?
> >
> > - Paul
> >
> >> On Oct 18, 2017, at 3:36 PM, julianhyde <gi...@git.apache.org> wrote:
> >>
> >> Github user julianhyde commented on a diff in the pull request:
> >>
> >> https://github.com/apache/drill/pull/984#discussion_r145561518
> >>
> >> --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java
> ---
> >> @@ -584,11 +492,14 @@ public static void defineWorkspace(Drillbit
> drillbit, String pluginName,
> >> public static final String EXPLAIN_PLAN_TEXT = "text";
> >> public static final String EXPLAIN_PLAN_JSON = "json";
> >>
> >> - public static FixtureBuilder builder() {
> >> - FixtureBuilder builder = new FixtureBuilder()
> >> + public static FixtureBuilder builder(DirTestWatcher
> dirTestWatcher) {
> >> --- End diff --
> >>
> >> Jeez Paul, please start a review rather than making single review
> comments. I just got 39 emails from you, and so did everyone else on
> dev@drill.
> >>
> >>
> >> ---
> >
>
>
Re: Excessive review comments
Posted by Julian Hyde <jh...@apache.org>.
I don’t know whether anything is broken. I believed that the GitHub “start a review” feature would cause all review comments to be sent in a single email. But now I think of it, I’m not sure I ever saw it working. I wonder whether Github-ASF integration is at fault.
Whatever the reasons for it, 39 emails to dev list is quite a blast. People tend to unsubscribe from lists if the volume is too high.
Julian
> On Oct 18, 2017, at 5:54 PM, Paul Rogers <pr...@mapr.com> wrote:
>
> With all due respect, I did start a review. Is something broken?
>
> - Paul
>
>> On Oct 18, 2017, at 3:36 PM, julianhyde <gi...@git.apache.org> wrote:
>>
>> Github user julianhyde commented on a diff in the pull request:
>>
>> https://github.com/apache/drill/pull/984#discussion_r145561518
>>
>> --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
>> @@ -584,11 +492,14 @@ public static void defineWorkspace(Drillbit drillbit, String pluginName,
>> public static final String EXPLAIN_PLAN_TEXT = "text";
>> public static final String EXPLAIN_PLAN_JSON = "json";
>>
>> - public static FixtureBuilder builder() {
>> - FixtureBuilder builder = new FixtureBuilder()
>> + public static FixtureBuilder builder(DirTestWatcher dirTestWatcher) {
>> --- End diff --
>>
>> Jeez Paul, please start a review rather than making single review comments. I just got 39 emails from you, and so did everyone else on dev@drill.
>>
>>
>> ---
>