drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul Rogers <prog...@mapr.com>
Subject Re: Excessive review comments
Date Thu, 19 Oct 2017 16:43:26 GMT
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 <jhyde@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 <progers@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 <git@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.
>>>> 
>>>> 
>>>> ---
>>> 
>> 
>> 

Mime
View raw message