Re: PR merge policy

2017-04-27 Thread Vlad Rozov
In this case please propose how to deal with PR merge policy violations 
in the future. I will -1 proposal to commit an improvement on top of a 
commit.


Thank you,

Vlad

On 4/27/17 21:48, Pramod Immaneni wrote:

I am sorry but I am -1 on the force push in this case.


On Apr 27, 2017, at 9:27 PM, Thomas Weise  wrote:

+1 as measure of last resort.


On Thu, Apr 27, 2017 at 9:25 PM, Vlad Rozov  wrote:

IMO, force push will bring enough consequent embarrassment to avoid such
behavior in the future.

Thank you,

Vlad


On 4/27/17 21:16, Munagala Ramanath wrote:

My thought was that leaving the bad commit would be a permanent reminder
to
the committer
(and others) that a policy violation occurred and the consequent
embarrassment would be an
adequate deterrent.

Ram

On Thu, Apr 27, 2017 at 9:12 PM, Vlad Rozov 
wrote:

I also was under impression that everyone agreed to the policy that gives

everyone in the community a chance to raise a concern or to propose an
improvement to a PR. Unfortunately, it is not the case, and we need to
discuss it again. I hope that this discussion will lead to no future
violations so we don't need to forcibly undo such commits, but it will be
good for the community to agree on the policy that deals with violations.

Ram, committing an improvement on top of a commit should be discouraged,
not encouraged as it eventually leads to the policy violation and lousy
PR
reviews.

Thank you,

Vlad

On 4/27/17 20:54, Thomas Weise wrote:

I also thought that everybody was in agreement about that after the first

round of discussion and as you say it would be hard to argue against it.
And I think we should not have to be back to the same topic a few days
later.

While you seem to be focussed on the disagreement on policy violation,
I'm
more interested in a style of collaboration that does not require such
discussion.

Thomas

On Thu, Apr 27, 2017 at 8:45 PM, Munagala Ramanath  wrote:

Forced push should not be necessary if committers follow the
development


process.

So why not focus on what the committer should do? Development process
and
guidelines are there for a reason, and the issue was raised before.

In this specific case, there is a string of commits related to the
plugin
feature that IMO should be part of the original review. There wasn't
any
need to rush this, the change wasn't important for the release.

Thomas


On Thu, Apr 27, 2017 at 8:11 PM, Munagala Ramanath <
r...@datatorrent.com
wrote:

I agree with Pramod that force pushing should be a rare event done
only


when there is an immediate
need to undo something serious. Doing it just for a policy violation

should

itself be codified in our

policies as a policy violation.

Why not just commit an improvement on top ?

Ram

On Thu, Apr 27, 2017 at 7:55 PM, Vlad Rozov 

wrote:
I am open to both approaches - two commits or a join commit. Both

have

pros and cons that we may discuss. What I am strongly against are
PRs

that

are merged without a chance for other contributors/committers to

review.

There should be a way to forcibly undo such commits.


Thank you,

Vlad


On 4/27/17 12:41, Pramod Immaneni wrote:

My comments inline..

On Thu, Apr 27, 2017 at 12:01 PM, Thomas Weise 

wrote:

I'm -1 on using the author field like this in Apex for the reason
stated
(it is also odd to see something like this showing up without
prior

discussion).

I am not set on this for future commits but would like to say,
do

we

really

verify the author field and treat it with importance. For

example, I

don't

think we ever check if the author is the person they say they are,

check

name, email etc. If I were to use slightly different variations of
my
name

or email (not that I would do that) would reviewers really verify

that.

I
also have checked that tools don't fail on reading a commit

because

author

needs to be in a certain format. I guess contribution stats are

the

ones

that will be 

[GitHub] apex-core pull request #521: APEXCORE-715 Remove unnecessary @Evolving annot...

2017-04-27 Thread vrozov
GitHub user vrozov opened a pull request:

https://github.com/apache/apex-core/pull/521

APEXCORE-715 Remove unnecessary @Evolving annotation in engine



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/vrozov/apex-core APEXCORE-715

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/apex-core/pull/521.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #521


commit ce0a135813012b989555e2c98203051c7596a15c
Author: Vlad Rozov 
Date:   2017-04-28T05:39:09Z

APEXCORE-715 Remove unnecessary @Evolving annotation in engine




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Commented] (APEXCORE-715) Remove unnecessary @Evolving annotation in engine

2017-04-27 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/APEXCORE-715?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15988250#comment-15988250
 ] 

ASF GitHub Bot commented on APEXCORE-715:
-

GitHub user vrozov opened a pull request:

https://github.com/apache/apex-core/pull/521

APEXCORE-715 Remove unnecessary @Evolving annotation in engine



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/vrozov/apex-core APEXCORE-715

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/apex-core/pull/521.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #521


commit ce0a135813012b989555e2c98203051c7596a15c
Author: Vlad Rozov 
Date:   2017-04-28T05:39:09Z

APEXCORE-715 Remove unnecessary @Evolving annotation in engine




> Remove unnecessary @Evolving annotation in engine
> -
>
> Key: APEXCORE-715
> URL: https://issues.apache.org/jira/browse/APEXCORE-715
> Project: Apache Apex Core
>  Issue Type: Improvement
>Reporter: Vlad Rozov
>Assignee: Vlad Rozov
>Priority: Minor
>
> \@Evolving annotation in the engine are not necessary and introduce 
> unnecessarily practice and wrong impression that some interfaces and/or 
> classes are more stable compared to others. All interfaces and classes in the 
> engine are not subject to the semantic version checks and may change from a 
> release to a release. The same applies to the buffer server.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Created] (APEXCORE-715) Remove unnecessary @Evolving annotation in engine

2017-04-27 Thread Vlad Rozov (JIRA)
Vlad Rozov created APEXCORE-715:
---

 Summary: Remove unnecessary @Evolving annotation in engine
 Key: APEXCORE-715
 URL: https://issues.apache.org/jira/browse/APEXCORE-715
 Project: Apache Apex Core
  Issue Type: Improvement
Reporter: Vlad Rozov
Assignee: Vlad Rozov
Priority: Minor


\@Evolving annotation in the engine are not necessary and introduce 
unnecessarily practice and wrong impression that some interfaces and/or classes 
are more stable compared to others. All interfaces and classes in the engine 
are not subject to the semantic version checks and may change from a release to 
a release. The same applies to the buffer server.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


Re: PR merge policy

2017-04-27 Thread Pramod Immaneni
I am sorry but I am -1 on the force push in this case.

> On Apr 27, 2017, at 9:27 PM, Thomas Weise  wrote:
>
> +1 as measure of last resort.
>
>> On Thu, Apr 27, 2017 at 9:25 PM, Vlad Rozov  wrote:
>>
>> IMO, force push will bring enough consequent embarrassment to avoid such
>> behavior in the future.
>>
>> Thank you,
>>
>> Vlad
>>
>>> On 4/27/17 21:16, Munagala Ramanath wrote:
>>>
>>> My thought was that leaving the bad commit would be a permanent reminder
>>> to
>>> the committer
>>> (and others) that a policy violation occurred and the consequent
>>> embarrassment would be an
>>> adequate deterrent.
>>>
>>> Ram
>>>
>>> On Thu, Apr 27, 2017 at 9:12 PM, Vlad Rozov 
>>> wrote:
>>>
>>> I also was under impression that everyone agreed to the policy that gives
 everyone in the community a chance to raise a concern or to propose an
 improvement to a PR. Unfortunately, it is not the case, and we need to
 discuss it again. I hope that this discussion will lead to no future
 violations so we don't need to forcibly undo such commits, but it will be
 good for the community to agree on the policy that deals with violations.

 Ram, committing an improvement on top of a commit should be discouraged,
 not encouraged as it eventually leads to the policy violation and lousy
 PR
 reviews.

 Thank you,

 Vlad

 On 4/27/17 20:54, Thomas Weise wrote:

 I also thought that everybody was in agreement about that after the first
> round of discussion and as you say it would be hard to argue against it.
> And I think we should not have to be back to the same topic a few days
> later.
>
> While you seem to be focussed on the disagreement on policy violation,
> I'm
> more interested in a style of collaboration that does not require such
> discussion.
>
> Thomas
>
> On Thu, Apr 27, 2017 at 8:45 PM, Munagala Ramanath >
> wrote:
>
> Everybody seems agreed on what the committers should do -- that waiting
> a
>
>> day or two for
>> others to have a chance to comment seems like an entirely reasonable
>> thing.
>>
>> The disagreement is about what to do when that policy is violated.
>>
>> Ram
>>
>> On Thu, Apr 27, 2017 at 8:37 PM, Thomas Weise  wrote:
>>
>> Forced push should not be necessary if committers follow the
>> development
>>
>>> process.
>>>
>>> So why not focus on what the committer should do? Development process
>>> and
>>> guidelines are there for a reason, and the issue was raised before.
>>>
>>> In this specific case, there is a string of commits related to the
>>> plugin
>>> feature that IMO should be part of the original review. There wasn't
>>> any
>>> need to rush this, the change wasn't important for the release.
>>>
>>> Thomas
>>>
>>>
>>> On Thu, Apr 27, 2017 at 8:11 PM, Munagala Ramanath <
>>> r...@datatorrent.com
>>> wrote:
>>>
>>> I agree with Pramod that force pushing should be a rare event done
>>> only
>>>
 when there is an immediate
 need to undo something serious. Doing it just for a policy violation

 should
>>>
>>> itself be codified in our
 policies as a policy violation.

 Why not just commit an improvement on top ?

 Ram

 On Thu, Apr 27, 2017 at 7:55 PM, Vlad Rozov 
 wrote:

 Violation of the PR merge policy is a sufficient reason to forcibly
 undo
 the commit and such violations affect everyone in the community.

> Thank you,
>
> Vlad
>
> On 4/27/17 19:36, Pramod Immaneni wrote:
>
> I agree that PRs should not be merged without a chance for others to
>
>> review. I don't agree to force push and altering the commit tree
>>
>> unless
>
 it

 is absolutely needed, as it affects everyone. This case doesn't
> warrant
>
 this step, one scenario where a force push might be needed is if

> somebody
> pushed some copyrighted code.
>
>> Thanks
>>
>> On Thu, Apr 27, 2017 at 6:44 PM, Vlad Rozov <
>>
>> v.ro...@datatorrent.com>
>
 wrote:
>>>
 I am open to both approaches - two commits or a join commit. Both
>>
>> have
>
 pros and cons that we may discuss. What I am strongly against are
>>>
 PRs
>>
> that
>>>
 are merged without a chance for other contributors/committers to
>>>
>>> review.
>>
> There should be a 

Re: PR merge policy

2017-04-27 Thread Pramod Immaneni
Let's move forward and discuss ideas that will help decrease the
occurrence of this or prevent it and not do the force commit which is
disruptive and in my opinion not needed for this situation.

Thomas whatever your opinion of the review, it was not the cause of
this. Second, like I mentioned earlier this was not developed as a
single feature, the way things happened, to have been captured
everything in one PR or review and there were two features that were
later unified. Your point about not requiring it for the release is
well taken.

> On Apr 27, 2017, at 9:16 PM, Munagala Ramanath  wrote:
>
> My thought was that leaving the bad commit would be a permanent reminder to
> the committer
> (and others) that a policy violation occurred and the consequent
> embarrassment would be an
> adequate deterrent.
>
> Ram
>
>> On Thu, Apr 27, 2017 at 9:12 PM, Vlad Rozov  wrote:
>>
>> I also was under impression that everyone agreed to the policy that gives
>> everyone in the community a chance to raise a concern or to propose an
>> improvement to a PR. Unfortunately, it is not the case, and we need to
>> discuss it again. I hope that this discussion will lead to no future
>> violations so we don't need to forcibly undo such commits, but it will be
>> good for the community to agree on the policy that deals with violations.
>>
>> Ram, committing an improvement on top of a commit should be discouraged,
>> not encouraged as it eventually leads to the policy violation and lousy PR
>> reviews.
>>
>> Thank you,
>>
>> Vlad
>>
>>> On 4/27/17 20:54, Thomas Weise wrote:
>>>
>>> I also thought that everybody was in agreement about that after the first
>>> round of discussion and as you say it would be hard to argue against it.
>>> And I think we should not have to be back to the same topic a few days
>>> later.
>>>
>>> While you seem to be focussed on the disagreement on policy violation, I'm
>>> more interested in a style of collaboration that does not require such
>>> discussion.
>>>
>>> Thomas
>>>
>>> On Thu, Apr 27, 2017 at 8:45 PM, Munagala Ramanath 
>>> wrote:
>>>
>>> Everybody seems agreed on what the committers should do -- that waiting a
 day or two for
 others to have a chance to comment seems like an entirely reasonable
 thing.

 The disagreement is about what to do when that policy is violated.

 Ram

 On Thu, Apr 27, 2017 at 8:37 PM, Thomas Weise  wrote:

 Forced push should not be necessary if committers follow the development
> process.
>
> So why not focus on what the committer should do? Development process
> and
> guidelines are there for a reason, and the issue was raised before.
>
> In this specific case, there is a string of commits related to the
> plugin
> feature that IMO should be part of the original review. There wasn't any
> need to rush this, the change wasn't important for the release.
>
> Thomas
>
>
> On Thu, Apr 27, 2017 at 8:11 PM, Munagala Ramanath >
> wrote:
>
> I agree with Pramod that force pushing should be a rare event done only
>> when there is an immediate
>> need to undo something serious. Doing it just for a policy violation
>>
> should
>
>> itself be codified in our
>> policies as a policy violation.
>>
>> Why not just commit an improvement on top ?
>>
>> Ram
>>
>> On Thu, Apr 27, 2017 at 7:55 PM, Vlad Rozov 
>> wrote:
>>
>> Violation of the PR merge policy is a sufficient reason to forcibly
>>>
>> undo
>
>> the commit and such violations affect everyone in the community.
>>>
>>> Thank you,
>>>
>>> Vlad
>>>
>>> On 4/27/17 19:36, Pramod Immaneni wrote:
>>>
>>> I agree that PRs should not be merged without a chance for others to
 review. I don't agree to force push and altering the commit tree

>>> unless
>
>> it
>>
>>> is absolutely needed, as it affects everyone. This case doesn't

>>> warrant
>
>> this step, one scenario where a force push might be needed is if

>>> somebody
>>
>>> pushed some copyrighted code.

 Thanks

 On Thu, Apr 27, 2017 at 6:44 PM, Vlad Rozov <

>>> v.ro...@datatorrent.com>

> wrote:

 I am open to both approaches - two commits or a join commit. Both

>>> have

> pros and cons that we may discuss. What I am strongly against are
>
 PRs

> that
> are merged without a chance for other contributors/committers to
>
 review.
>>
>>> There should be a way to forcibly undo such commits.
>
> Thank you,
>
> Vlad
>
>
> On 4/27/17 12:41, Pramod 

Re: Join support in Malhar

2017-04-27 Thread Thomas Weise
There is one more important difference not mentioned:

Join Impl 1 doesn't work and Join Impl 2 does :)

Can you clarify why a (working) Join Impl 1 would perform better? And if it
is the case, how the amount of work fixing 1 would stack up against
improving 2?

Join Impl 2 has greater flexibility due to the generalized windowing. If
everything else is same I prefer we put our efforts there.

Thanks,
Thomas



On Wed, Apr 26, 2017 at 11:14 PM, Bhupesh Chawda  wrote:

> Hi Community,
>
> Currently the support for join in Malhar is little fuzzy for the end user.
> We have multiple implementations -
>
>1. Join Impl 1 - Inner Join implementation, based on Managed state
>2. Join Impl 2 - Merge operator, Windowed implementation, based on
>Spillable structures (based on managed state)
>
> Following are the differences between the two:
>
>- As the name implies, Join Impl 1 is meant for inner joins, while Join
>Impl 2 has generic support for inner as well as outer joins.
>- Join Impl 1 supports sliding time windows with support for expiring
>old tuples. Join Impl 2 needs understanding of windowing concepts and
> uses
>watermarking support for functioning.
>- By looking at the implementations of managed state used by Join Impl 1
>and Join Impl 2, it seems like Join Impl 1 would have a performance
>advantage over Join Impl 2.
>
> The purpose of this email is to see what can be done to simplify the join
> usability in Malhar. Following are some options:
>
>1. Keep both implementations with clear documentation of the usability
>for both.
>2. Remove Join Impl 1 from Malhar and work with Join Impl 2 to improve
>performance. Note that even though Join Impl 1 addresses a very specific
>use case, it is the most common requirement in streaming join use cases.
>3. Any other option?
>
> Thanks.
>
> ~ Bhupesh
>
> ​​
>


Re: PR merge policy

2017-04-27 Thread Thomas Weise
+1 as measure of last resort.

On Thu, Apr 27, 2017 at 9:25 PM, Vlad Rozov  wrote:

> IMO, force push will bring enough consequent embarrassment to avoid such
> behavior in the future.
>
> Thank you,
>
> Vlad
>
> On 4/27/17 21:16, Munagala Ramanath wrote:
>
>> My thought was that leaving the bad commit would be a permanent reminder
>> to
>> the committer
>> (and others) that a policy violation occurred and the consequent
>> embarrassment would be an
>> adequate deterrent.
>>
>> Ram
>>
>> On Thu, Apr 27, 2017 at 9:12 PM, Vlad Rozov 
>> wrote:
>>
>> I also was under impression that everyone agreed to the policy that gives
>>> everyone in the community a chance to raise a concern or to propose an
>>> improvement to a PR. Unfortunately, it is not the case, and we need to
>>> discuss it again. I hope that this discussion will lead to no future
>>> violations so we don't need to forcibly undo such commits, but it will be
>>> good for the community to agree on the policy that deals with violations.
>>>
>>> Ram, committing an improvement on top of a commit should be discouraged,
>>> not encouraged as it eventually leads to the policy violation and lousy
>>> PR
>>> reviews.
>>>
>>> Thank you,
>>>
>>> Vlad
>>>
>>> On 4/27/17 20:54, Thomas Weise wrote:
>>>
>>> I also thought that everybody was in agreement about that after the first
 round of discussion and as you say it would be hard to argue against it.
 And I think we should not have to be back to the same topic a few days
 later.

 While you seem to be focussed on the disagreement on policy violation,
 I'm
 more interested in a style of collaboration that does not require such
 discussion.

 Thomas

 On Thu, Apr 27, 2017 at 8:45 PM, Munagala Ramanath 
 wrote:

 Everybody seems agreed on what the committers should do -- that waiting
 a

> day or two for
> others to have a chance to comment seems like an entirely reasonable
> thing.
>
> The disagreement is about what to do when that policy is violated.
>
> Ram
>
> On Thu, Apr 27, 2017 at 8:37 PM, Thomas Weise  wrote:
>
> Forced push should not be necessary if committers follow the
> development
>
>> process.
>>
>> So why not focus on what the committer should do? Development process
>> and
>> guidelines are there for a reason, and the issue was raised before.
>>
>> In this specific case, there is a string of commits related to the
>> plugin
>> feature that IMO should be part of the original review. There wasn't
>> any
>> need to rush this, the change wasn't important for the release.
>>
>> Thomas
>>
>>
>> On Thu, Apr 27, 2017 at 8:11 PM, Munagala Ramanath <
>> r...@datatorrent.com
>> wrote:
>>
>> I agree with Pramod that force pushing should be a rare event done
>> only
>>
>>> when there is an immediate
>>> need to undo something serious. Doing it just for a policy violation
>>>
>>> should
>>
>> itself be codified in our
>>> policies as a policy violation.
>>>
>>> Why not just commit an improvement on top ?
>>>
>>> Ram
>>>
>>> On Thu, Apr 27, 2017 at 7:55 PM, Vlad Rozov >> >
>>> wrote:
>>>
>>> Violation of the PR merge policy is a sufficient reason to forcibly
>>> undo
>>> the commit and such violations affect everyone in the community.
>>>
 Thank you,

 Vlad

 On 4/27/17 19:36, Pramod Immaneni wrote:

 I agree that PRs should not be merged without a chance for others to

> review. I don't agree to force push and altering the commit tree
>
> unless

>>> it
>>>
>>> is absolutely needed, as it affects everyone. This case doesn't
 warrant

>>> this step, one scenario where a force push might be needed is if
>>>
 somebody
 pushed some copyrighted code.

> Thanks
>
> On Thu, Apr 27, 2017 at 6:44 PM, Vlad Rozov <
>
> v.ro...@datatorrent.com>

>>> wrote:
>>
>>> I am open to both approaches - two commits or a join commit. Both
>
> have

>>> pros and cons that we may discuss. What I am strongly against are
>>
>>> PRs
>
 that
>>
>>> are merged without a chance for other contributors/committers to
>>
>> review.
>
 There should be a way to forcibly undo such commits.

> Thank you,
>>
>> Vlad
>>
>>
>> On 4/27/17 12:41, Pramod Immaneni wrote:
>>
>> My comments inline..
>>
>> On Thu, Apr 27, 2017 at 12:01 PM, Thomas Weise 

Re: PR merge policy

2017-04-27 Thread Thomas Weise
That's an interesting thought also. Here is one more: Committer on the
project should require demonstrated responsible behavior in this area.
Proven ability to collaborate and doing the right things will be a criteria
for such nominations on my list.


On Thu, Apr 27, 2017 at 9:16 PM, Munagala Ramanath 
wrote:

> My thought was that leaving the bad commit would be a permanent reminder to
> the committer
> (and others) that a policy violation occurred and the consequent
> embarrassment would be an
> adequate deterrent.
>
> Ram
>
> On Thu, Apr 27, 2017 at 9:12 PM, Vlad Rozov 
> wrote:
>
> > I also was under impression that everyone agreed to the policy that gives
> > everyone in the community a chance to raise a concern or to propose an
> > improvement to a PR. Unfortunately, it is not the case, and we need to
> > discuss it again. I hope that this discussion will lead to no future
> > violations so we don't need to forcibly undo such commits, but it will be
> > good for the community to agree on the policy that deals with violations.
> >
> > Ram, committing an improvement on top of a commit should be discouraged,
> > not encouraged as it eventually leads to the policy violation and lousy
> PR
> > reviews.
> >
> > Thank you,
> >
> > Vlad
> >
> > On 4/27/17 20:54, Thomas Weise wrote:
> >
> >> I also thought that everybody was in agreement about that after the
> first
> >> round of discussion and as you say it would be hard to argue against it.
> >> And I think we should not have to be back to the same topic a few days
> >> later.
> >>
> >> While you seem to be focussed on the disagreement on policy violation,
> I'm
> >> more interested in a style of collaboration that does not require such
> >> discussion.
> >>
> >> Thomas
> >>
> >> On Thu, Apr 27, 2017 at 8:45 PM, Munagala Ramanath  >
> >> wrote:
> >>
> >> Everybody seems agreed on what the committers should do -- that waiting
> a
> >>> day or two for
> >>> others to have a chance to comment seems like an entirely reasonable
> >>> thing.
> >>>
> >>> The disagreement is about what to do when that policy is violated.
> >>>
> >>> Ram
> >>>
> >>> On Thu, Apr 27, 2017 at 8:37 PM, Thomas Weise  wrote:
> >>>
> >>> Forced push should not be necessary if committers follow the
> development
>  process.
> 
>  So why not focus on what the committer should do? Development process
>  and
>  guidelines are there for a reason, and the issue was raised before.
> 
>  In this specific case, there is a string of commits related to the
>  plugin
>  feature that IMO should be part of the original review. There wasn't
> any
>  need to rush this, the change wasn't important for the release.
> 
>  Thomas
> 
> 
>  On Thu, Apr 27, 2017 at 8:11 PM, Munagala Ramanath <
> r...@datatorrent.com
>  >
>  wrote:
> 
>  I agree with Pramod that force pushing should be a rare event done
> only
> > when there is an immediate
> > need to undo something serious. Doing it just for a policy violation
> >
>  should
> 
> > itself be codified in our
> > policies as a policy violation.
> >
> > Why not just commit an improvement on top ?
> >
> > Ram
> >
> > On Thu, Apr 27, 2017 at 7:55 PM, Vlad Rozov  >
> > wrote:
> >
> > Violation of the PR merge policy is a sufficient reason to forcibly
> >>
> > undo
> 
> > the commit and such violations affect everyone in the community.
> >>
> >> Thank you,
> >>
> >> Vlad
> >>
> >> On 4/27/17 19:36, Pramod Immaneni wrote:
> >>
> >> I agree that PRs should not be merged without a chance for others to
> >>> review. I don't agree to force push and altering the commit tree
> >>>
> >> unless
> 
> > it
> >
> >> is absolutely needed, as it affects everyone. This case doesn't
> >>>
> >> warrant
> 
> > this step, one scenario where a force push might be needed is if
> >>>
> >> somebody
> >
> >> pushed some copyrighted code.
> >>>
> >>> Thanks
> >>>
> >>> On Thu, Apr 27, 2017 at 6:44 PM, Vlad Rozov <
> >>>
> >> v.ro...@datatorrent.com>
> >>>
>  wrote:
> >>>
> >>> I am open to both approaches - two commits or a join commit. Both
> >>>
> >> have
> >>>
>  pros and cons that we may discuss. What I am strongly against are
> 
> >>> PRs
> >>>
>  that
>  are merged without a chance for other contributors/committers to
> 
> >>> review.
> >
> >> There should be a way to forcibly undo such commits.
> 
>  Thank you,
> 
>  Vlad
> 
> 
>  On 4/27/17 12:41, Pramod Immaneni wrote:
> 
>  My comments inline..
> 
> > On Thu, Apr 27, 2017 at 12:01 PM, Thomas Weise 

Re: PR merge policy

2017-04-27 Thread Vlad Rozov
IMO, force push will bring enough consequent embarrassment to avoid such 
behavior in the future.


Thank you,

Vlad
On 4/27/17 21:16, Munagala Ramanath wrote:

My thought was that leaving the bad commit would be a permanent reminder to
the committer
(and others) that a policy violation occurred and the consequent
embarrassment would be an
adequate deterrent.

Ram

On Thu, Apr 27, 2017 at 9:12 PM, Vlad Rozov  wrote:


I also was under impression that everyone agreed to the policy that gives
everyone in the community a chance to raise a concern or to propose an
improvement to a PR. Unfortunately, it is not the case, and we need to
discuss it again. I hope that this discussion will lead to no future
violations so we don't need to forcibly undo such commits, but it will be
good for the community to agree on the policy that deals with violations.

Ram, committing an improvement on top of a commit should be discouraged,
not encouraged as it eventually leads to the policy violation and lousy PR
reviews.

Thank you,

Vlad

On 4/27/17 20:54, Thomas Weise wrote:


I also thought that everybody was in agreement about that after the first
round of discussion and as you say it would be hard to argue against it.
And I think we should not have to be back to the same topic a few days
later.

While you seem to be focussed on the disagreement on policy violation, I'm
more interested in a style of collaboration that does not require such
discussion.

Thomas

On Thu, Apr 27, 2017 at 8:45 PM, Munagala Ramanath 
wrote:

Everybody seems agreed on what the committers should do -- that waiting a

day or two for
others to have a chance to comment seems like an entirely reasonable
thing.

The disagreement is about what to do when that policy is violated.

Ram

On Thu, Apr 27, 2017 at 8:37 PM, Thomas Weise  wrote:

Forced push should not be necessary if committers follow the development

process.

So why not focus on what the committer should do? Development process
and
guidelines are there for a reason, and the issue was raised before.

In this specific case, there is a string of commits related to the
plugin
feature that IMO should be part of the original review. There wasn't any
need to rush this, the change wasn't important for the release.

Thomas


On Thu, Apr 27, 2017 at 8:11 PM, Munagala Ramanath 
wrote:

Violation of the PR merge policy is a sufficient reason to forcibly
undo
the commit and such violations affect everyone in the community.

Thank you,

Vlad

On 4/27/17 19:36, Pramod Immaneni wrote:

I agree that PRs should not be merged without a chance for others to

review. I don't agree to force push and altering the commit tree


unless

it


is absolutely needed, as it affects everyone. This case doesn't
warrant

this step, one scenario where a force push might be needed is if

somebody
pushed some copyrighted code.

Thanks

On Thu, Apr 27, 2017 at 6:44 PM, Vlad Rozov <


v.ro...@datatorrent.com>

wrote:

I am open to both approaches - two commits or a join commit. Both


have

pros and cons that we may discuss. What I am strongly against are

PRs

that

are merged without a chance for other contributors/committers to


review.

There should be a way to forcibly undo such commits.

Thank you,

Vlad


On 4/27/17 12:41, Pramod Immaneni wrote:

My comments inline..


On Thu, Apr 27, 2017 at 12:01 PM, Thomas Weise 


wrote:

I'm -1 on using the author field like this in Apex for the reason

stated

(it is also odd to see something like this showing up without

prior

discussion).


I am not set on this for future commits but would like to say, do


we

really

verify the author field and treat it with importance. For


example, I

don't

think we ever check if the author is the person they say they are,


check

name, email etc. If I were to use slightly different variations of

my

name

or email (not that I would do that) would reviewers really verify


that.

I

also have checked that tools don't fail on reading a commit


because

author

needs to be in a certain format. I guess contribution stats are


the

ones

that will be affected but if used rarely I dont see that being a

big

problem. I can understand if we wanted to have strict requirements

for

the

committer field.

Thanks


Consider adding the additional author information to the commit


message.

Thomas

On Thu, Apr 27, 2017 at 11:55 AM, Pramod Immaneni <
pra...@datatorrent.com>
wrote:

Agreed it is not regular and should only be used in special
circumstances.

One example of this is pair 

Re: PR merge policy

2017-04-27 Thread Munagala Ramanath
My thought was that leaving the bad commit would be a permanent reminder to
the committer
(and others) that a policy violation occurred and the consequent
embarrassment would be an
adequate deterrent.

Ram

On Thu, Apr 27, 2017 at 9:12 PM, Vlad Rozov  wrote:

> I also was under impression that everyone agreed to the policy that gives
> everyone in the community a chance to raise a concern or to propose an
> improvement to a PR. Unfortunately, it is not the case, and we need to
> discuss it again. I hope that this discussion will lead to no future
> violations so we don't need to forcibly undo such commits, but it will be
> good for the community to agree on the policy that deals with violations.
>
> Ram, committing an improvement on top of a commit should be discouraged,
> not encouraged as it eventually leads to the policy violation and lousy PR
> reviews.
>
> Thank you,
>
> Vlad
>
> On 4/27/17 20:54, Thomas Weise wrote:
>
>> I also thought that everybody was in agreement about that after the first
>> round of discussion and as you say it would be hard to argue against it.
>> And I think we should not have to be back to the same topic a few days
>> later.
>>
>> While you seem to be focussed on the disagreement on policy violation, I'm
>> more interested in a style of collaboration that does not require such
>> discussion.
>>
>> Thomas
>>
>> On Thu, Apr 27, 2017 at 8:45 PM, Munagala Ramanath 
>> wrote:
>>
>> Everybody seems agreed on what the committers should do -- that waiting a
>>> day or two for
>>> others to have a chance to comment seems like an entirely reasonable
>>> thing.
>>>
>>> The disagreement is about what to do when that policy is violated.
>>>
>>> Ram
>>>
>>> On Thu, Apr 27, 2017 at 8:37 PM, Thomas Weise  wrote:
>>>
>>> Forced push should not be necessary if committers follow the development
 process.

 So why not focus on what the committer should do? Development process
 and
 guidelines are there for a reason, and the issue was raised before.

 In this specific case, there is a string of commits related to the
 plugin
 feature that IMO should be part of the original review. There wasn't any
 need to rush this, the change wasn't important for the release.

 Thomas


 On Thu, Apr 27, 2017 at 8:11 PM, Munagala Ramanath 
 wrote:

 I agree with Pramod that force pushing should be a rare event done only
> when there is an immediate
> need to undo something serious. Doing it just for a policy violation
>
 should

> itself be codified in our
> policies as a policy violation.
>
> Why not just commit an improvement on top ?
>
> Ram
>
> On Thu, Apr 27, 2017 at 7:55 PM, Vlad Rozov 
> wrote:
>
> Violation of the PR merge policy is a sufficient reason to forcibly
>>
> undo

> the commit and such violations affect everyone in the community.
>>
>> Thank you,
>>
>> Vlad
>>
>> On 4/27/17 19:36, Pramod Immaneni wrote:
>>
>> I agree that PRs should not be merged without a chance for others to
>>> review. I don't agree to force push and altering the commit tree
>>>
>> unless

> it
>
>> is absolutely needed, as it affects everyone. This case doesn't
>>>
>> warrant

> this step, one scenario where a force push might be needed is if
>>>
>> somebody
>
>> pushed some copyrighted code.
>>>
>>> Thanks
>>>
>>> On Thu, Apr 27, 2017 at 6:44 PM, Vlad Rozov <
>>>
>> v.ro...@datatorrent.com>
>>>
 wrote:
>>>
>>> I am open to both approaches - two commits or a join commit. Both
>>>
>> have
>>>
 pros and cons that we may discuss. What I am strongly against are

>>> PRs
>>>
 that
 are merged without a chance for other contributors/committers to

>>> review.
>
>> There should be a way to forcibly undo such commits.

 Thank you,

 Vlad


 On 4/27/17 12:41, Pramod Immaneni wrote:

 My comments inline..

> On Thu, Apr 27, 2017 at 12:01 PM, Thomas Weise 
>
 wrote:
>
>> I'm -1 on using the author field like this in Apex for the reason
>
 stated
>
>> (it is also odd to see something like this showing up without
>>
> prior
>>>
 discussion).
>>
>>
>> I am not set on this for future commits but would like to say, do
>>
> we

> really
> verify the author field and treat it with importance. For
>
 example, I
>>>
 don't
> think we ever check if the author is the person they say they are,
>
 check
>
>> name, email etc. 

Re: PR merge policy

2017-04-27 Thread Thomas Weise
Haha. Community won't grow without idealism.


On Thu, Apr 27, 2017 at 9:03 PM, Munagala Ramanath 
wrote:

> Idealism is a wonderful thing but reality sometimes intrudes.
>
> Ram
>
> On Thu, Apr 27, 2017 at 8:54 PM, Thomas Weise  wrote:
>
> > I also thought that everybody was in agreement about that after the first
> > round of discussion and as you say it would be hard to argue against it.
> > And I think we should not have to be back to the same topic a few days
> > later.
> >
> > While you seem to be focussed on the disagreement on policy violation,
> I'm
> > more interested in a style of collaboration that does not require such
> > discussion.
> >
> > Thomas
> >
> > On Thu, Apr 27, 2017 at 8:45 PM, Munagala Ramanath 
> > wrote:
> >
> > > Everybody seems agreed on what the committers should do -- that
> waiting a
> > > day or two for
> > > others to have a chance to comment seems like an entirely reasonable
> > thing.
> > >
> > > The disagreement is about what to do when that policy is violated.
> > >
> > > Ram
> > >
> > > On Thu, Apr 27, 2017 at 8:37 PM, Thomas Weise  wrote:
> > >
> > > > Forced push should not be necessary if committers follow the
> > development
> > > > process.
> > > >
> > > > So why not focus on what the committer should do? Development process
> > and
> > > > guidelines are there for a reason, and the issue was raised before.
> > > >
> > > > In this specific case, there is a string of commits related to the
> > plugin
> > > > feature that IMO should be part of the original review. There wasn't
> > any
> > > > need to rush this, the change wasn't important for the release.
> > > >
> > > > Thomas
> > > >
> > > >
> > > > On Thu, Apr 27, 2017 at 8:11 PM, Munagala Ramanath <
> > r...@datatorrent.com>
> > > > wrote:
> > > >
> > > > > I agree with Pramod that force pushing should be a rare event done
> > only
> > > > > when there is an immediate
> > > > > need to undo something serious. Doing it just for a policy
> violation
> > > > should
> > > > > itself be codified in our
> > > > > policies as a policy violation.
> > > > >
> > > > > Why not just commit an improvement on top ?
> > > > >
> > > > > Ram
> > > > >
> > > > > On Thu, Apr 27, 2017 at 7:55 PM, Vlad Rozov <
> v.ro...@datatorrent.com
> > >
> > > > > wrote:
> > > > >
> > > > > > Violation of the PR merge policy is a sufficient reason to
> forcibly
> > > > undo
> > > > > > the commit and such violations affect everyone in the community.
> > > > > >
> > > > > > Thank you,
> > > > > >
> > > > > > Vlad
> > > > > >
> > > > > > On 4/27/17 19:36, Pramod Immaneni wrote:
> > > > > >
> > > > > >> I agree that PRs should not be merged without a chance for
> others
> > to
> > > > > >> review. I don't agree to force push and altering the commit tree
> > > > unless
> > > > > it
> > > > > >> is absolutely needed, as it affects everyone. This case doesn't
> > > > warrant
> > > > > >> this step, one scenario where a force push might be needed is if
> > > > > somebody
> > > > > >> pushed some copyrighted code.
> > > > > >>
> > > > > >> Thanks
> > > > > >>
> > > > > >> On Thu, Apr 27, 2017 at 6:44 PM, Vlad Rozov <
> > > v.ro...@datatorrent.com>
> > > > > >> wrote:
> > > > > >>
> > > > > >> I am open to both approaches - two commits or a join commit.
> Both
> > > have
> > > > > >>> pros and cons that we may discuss. What I am strongly against
> are
> > > PRs
> > > > > >>> that
> > > > > >>> are merged without a chance for other contributors/committers
> to
> > > > > review.
> > > > > >>> There should be a way to forcibly undo such commits.
> > > > > >>>
> > > > > >>> Thank you,
> > > > > >>>
> > > > > >>> Vlad
> > > > > >>>
> > > > > >>>
> > > > > >>> On 4/27/17 12:41, Pramod Immaneni wrote:
> > > > > >>>
> > > > > >>> My comments inline..
> > > > > 
> > > > >  On Thu, Apr 27, 2017 at 12:01 PM, Thomas Weise <
> t...@apache.org>
> > > > > wrote:
> > > > > 
> > > > >  I'm -1 on using the author field like this in Apex for the
> > reason
> > > > > stated
> > > > > 
> > > > > > (it is also odd to see something like this showing up without
> > > prior
> > > > > > discussion).
> > > > > >
> > > > > >
> > > > > > I am not set on this for future commits but would like to
> say,
> > do
> > > > we
> > > > > >
> > > > >  really
> > > > >  verify the author field and treat it with importance. For
> > > example, I
> > > > >  don't
> > > > >  think we ever check if the author is the person they say they
> > are,
> > > > > check
> > > > >  name, email etc. If I were to use slightly different
> variations
> > of
> > > > my
> > > > >  name
> > > > >  or email (not that I would do that) would reviewers really
> > verify
> > > > > that.
> > > > >  I
> > > > >  also have checked that tools don't fail on reading a commit
> > > because
> > > > >  author
> > > > >  needs to be in a certain format. I guess 

Re: PR merge policy

2017-04-27 Thread Vlad Rozov
I also was under impression that everyone agreed to the policy that 
gives everyone in the community a chance to raise a concern or to 
propose an improvement to a PR. Unfortunately, it is not the case, and 
we need to discuss it again. I hope that this discussion will lead to no 
future violations so we don't need to forcibly undo such commits, but it 
will be good for the community to agree on the policy that deals with 
violations.


Ram, committing an improvement on top of a commit should be discouraged, 
not encouraged as it eventually leads to the policy violation and lousy 
PR reviews.


Thank you,

Vlad

On 4/27/17 20:54, Thomas Weise wrote:

I also thought that everybody was in agreement about that after the first
round of discussion and as you say it would be hard to argue against it.
And I think we should not have to be back to the same topic a few days
later.

While you seem to be focussed on the disagreement on policy violation, I'm
more interested in a style of collaboration that does not require such
discussion.

Thomas

On Thu, Apr 27, 2017 at 8:45 PM, Munagala Ramanath 
wrote:


Everybody seems agreed on what the committers should do -- that waiting a
day or two for
others to have a chance to comment seems like an entirely reasonable thing.

The disagreement is about what to do when that policy is violated.

Ram

On Thu, Apr 27, 2017 at 8:37 PM, Thomas Weise  wrote:


Forced push should not be necessary if committers follow the development
process.

So why not focus on what the committer should do? Development process and
guidelines are there for a reason, and the issue was raised before.

In this specific case, there is a string of commits related to the plugin
feature that IMO should be part of the original review. There wasn't any
need to rush this, the change wasn't important for the release.

Thomas


On Thu, Apr 27, 2017 at 8:11 PM, Munagala Ramanath 
wrote:


I agree with Pramod that force pushing should be a rare event done only
when there is an immediate
need to undo something serious. Doing it just for a policy violation

should

itself be codified in our
policies as a policy violation.

Why not just commit an improvement on top ?

Ram

On Thu, Apr 27, 2017 at 7:55 PM, Vlad Rozov 
wrote:


Violation of the PR merge policy is a sufficient reason to forcibly

undo

the commit and such violations affect everyone in the community.

Thank you,

Vlad

On 4/27/17 19:36, Pramod Immaneni wrote:


I agree that PRs should not be merged without a chance for others to
review. I don't agree to force push and altering the commit tree

unless

it

is absolutely needed, as it affects everyone. This case doesn't

warrant

this step, one scenario where a force push might be needed is if

somebody

pushed some copyrighted code.

Thanks

On Thu, Apr 27, 2017 at 6:44 PM, Vlad Rozov <

v.ro...@datatorrent.com>

wrote:

I am open to both approaches - two commits or a join commit. Both

have

pros and cons that we may discuss. What I am strongly against are

PRs

that
are merged without a chance for other contributors/committers to

review.

There should be a way to forcibly undo such commits.

Thank you,

Vlad


On 4/27/17 12:41, Pramod Immaneni wrote:

My comments inline..

On Thu, Apr 27, 2017 at 12:01 PM, Thomas Weise 

wrote:

I'm -1 on using the author field like this in Apex for the reason

stated

(it is also odd to see something like this showing up without

prior

discussion).


I am not set on this for future commits but would like to say, do

we

really
verify the author field and treat it with importance. For

example, I

don't
think we ever check if the author is the person they say they are,

check

name, email etc. If I were to use slightly different variations of

my

name
or email (not that I would do that) would reviewers really verify

that.

I
also have checked that tools don't fail on reading a commit

because

author
needs to be in a certain format. I guess contribution stats are

the

ones

that will be affected but if used rarely I dont see that being a

big

problem. I can understand if we wanted to have strict requirements

for

the
committer field.

Thanks


Consider adding the additional author information to the commit

message.

Thomas

On Thu, Apr 27, 2017 at 11:55 AM, Pramod Immaneni <
pra...@datatorrent.com>
wrote:

Agreed it is not regular and should only be used in special
circumstances.

One example of this is pair programming. It has been done before

in

other
projects and searching on google or stackoverflow you can see

how

other
people have tried to handle it

https://bugs.eclipse.org/bugs/show_bug.cgi?id=375536
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=451880
http://stackoverflow.com/questions/7442112/attributing-
a-single-commit-to-multiple-developers

Thanks

On Thu, Apr 27, 2017 at 9:57 AM, Thomas Weise 

wrote:

commit 

Re: PR merge policy

2017-04-27 Thread Munagala Ramanath
Idealism is a wonderful thing but reality sometimes intrudes.

Ram

On Thu, Apr 27, 2017 at 8:54 PM, Thomas Weise  wrote:

> I also thought that everybody was in agreement about that after the first
> round of discussion and as you say it would be hard to argue against it.
> And I think we should not have to be back to the same topic a few days
> later.
>
> While you seem to be focussed on the disagreement on policy violation, I'm
> more interested in a style of collaboration that does not require such
> discussion.
>
> Thomas
>
> On Thu, Apr 27, 2017 at 8:45 PM, Munagala Ramanath 
> wrote:
>
> > Everybody seems agreed on what the committers should do -- that waiting a
> > day or two for
> > others to have a chance to comment seems like an entirely reasonable
> thing.
> >
> > The disagreement is about what to do when that policy is violated.
> >
> > Ram
> >
> > On Thu, Apr 27, 2017 at 8:37 PM, Thomas Weise  wrote:
> >
> > > Forced push should not be necessary if committers follow the
> development
> > > process.
> > >
> > > So why not focus on what the committer should do? Development process
> and
> > > guidelines are there for a reason, and the issue was raised before.
> > >
> > > In this specific case, there is a string of commits related to the
> plugin
> > > feature that IMO should be part of the original review. There wasn't
> any
> > > need to rush this, the change wasn't important for the release.
> > >
> > > Thomas
> > >
> > >
> > > On Thu, Apr 27, 2017 at 8:11 PM, Munagala Ramanath <
> r...@datatorrent.com>
> > > wrote:
> > >
> > > > I agree with Pramod that force pushing should be a rare event done
> only
> > > > when there is an immediate
> > > > need to undo something serious. Doing it just for a policy violation
> > > should
> > > > itself be codified in our
> > > > policies as a policy violation.
> > > >
> > > > Why not just commit an improvement on top ?
> > > >
> > > > Ram
> > > >
> > > > On Thu, Apr 27, 2017 at 7:55 PM, Vlad Rozov  >
> > > > wrote:
> > > >
> > > > > Violation of the PR merge policy is a sufficient reason to forcibly
> > > undo
> > > > > the commit and such violations affect everyone in the community.
> > > > >
> > > > > Thank you,
> > > > >
> > > > > Vlad
> > > > >
> > > > > On 4/27/17 19:36, Pramod Immaneni wrote:
> > > > >
> > > > >> I agree that PRs should not be merged without a chance for others
> to
> > > > >> review. I don't agree to force push and altering the commit tree
> > > unless
> > > > it
> > > > >> is absolutely needed, as it affects everyone. This case doesn't
> > > warrant
> > > > >> this step, one scenario where a force push might be needed is if
> > > > somebody
> > > > >> pushed some copyrighted code.
> > > > >>
> > > > >> Thanks
> > > > >>
> > > > >> On Thu, Apr 27, 2017 at 6:44 PM, Vlad Rozov <
> > v.ro...@datatorrent.com>
> > > > >> wrote:
> > > > >>
> > > > >> I am open to both approaches - two commits or a join commit. Both
> > have
> > > > >>> pros and cons that we may discuss. What I am strongly against are
> > PRs
> > > > >>> that
> > > > >>> are merged without a chance for other contributors/committers to
> > > > review.
> > > > >>> There should be a way to forcibly undo such commits.
> > > > >>>
> > > > >>> Thank you,
> > > > >>>
> > > > >>> Vlad
> > > > >>>
> > > > >>>
> > > > >>> On 4/27/17 12:41, Pramod Immaneni wrote:
> > > > >>>
> > > > >>> My comments inline..
> > > > 
> > > >  On Thu, Apr 27, 2017 at 12:01 PM, Thomas Weise 
> > > > wrote:
> > > > 
> > > >  I'm -1 on using the author field like this in Apex for the
> reason
> > > > stated
> > > > 
> > > > > (it is also odd to see something like this showing up without
> > prior
> > > > > discussion).
> > > > >
> > > > >
> > > > > I am not set on this for future commits but would like to say,
> do
> > > we
> > > > >
> > > >  really
> > > >  verify the author field and treat it with importance. For
> > example, I
> > > >  don't
> > > >  think we ever check if the author is the person they say they
> are,
> > > > check
> > > >  name, email etc. If I were to use slightly different variations
> of
> > > my
> > > >  name
> > > >  or email (not that I would do that) would reviewers really
> verify
> > > > that.
> > > >  I
> > > >  also have checked that tools don't fail on reading a commit
> > because
> > > >  author
> > > >  needs to be in a certain format. I guess contribution stats are
> > the
> > > > ones
> > > >  that will be affected but if used rarely I dont see that being a
> > big
> > > >  problem. I can understand if we wanted to have strict
> requirements
> > > for
> > > >  the
> > > >  committer field.
> > > > 
> > > >  Thanks
> > > > 
> > > > 
> > > >  Consider adding the additional author information to the commit
> > > > message.
> > > > 
> > > > > 

Re: PR merge policy

2017-04-27 Thread Thomas Weise
I also thought that everybody was in agreement about that after the first
round of discussion and as you say it would be hard to argue against it.
And I think we should not have to be back to the same topic a few days
later.

While you seem to be focussed on the disagreement on policy violation, I'm
more interested in a style of collaboration that does not require such
discussion.

Thomas

On Thu, Apr 27, 2017 at 8:45 PM, Munagala Ramanath 
wrote:

> Everybody seems agreed on what the committers should do -- that waiting a
> day or two for
> others to have a chance to comment seems like an entirely reasonable thing.
>
> The disagreement is about what to do when that policy is violated.
>
> Ram
>
> On Thu, Apr 27, 2017 at 8:37 PM, Thomas Weise  wrote:
>
> > Forced push should not be necessary if committers follow the development
> > process.
> >
> > So why not focus on what the committer should do? Development process and
> > guidelines are there for a reason, and the issue was raised before.
> >
> > In this specific case, there is a string of commits related to the plugin
> > feature that IMO should be part of the original review. There wasn't any
> > need to rush this, the change wasn't important for the release.
> >
> > Thomas
> >
> >
> > On Thu, Apr 27, 2017 at 8:11 PM, Munagala Ramanath 
> > wrote:
> >
> > > I agree with Pramod that force pushing should be a rare event done only
> > > when there is an immediate
> > > need to undo something serious. Doing it just for a policy violation
> > should
> > > itself be codified in our
> > > policies as a policy violation.
> > >
> > > Why not just commit an improvement on top ?
> > >
> > > Ram
> > >
> > > On Thu, Apr 27, 2017 at 7:55 PM, Vlad Rozov 
> > > wrote:
> > >
> > > > Violation of the PR merge policy is a sufficient reason to forcibly
> > undo
> > > > the commit and such violations affect everyone in the community.
> > > >
> > > > Thank you,
> > > >
> > > > Vlad
> > > >
> > > > On 4/27/17 19:36, Pramod Immaneni wrote:
> > > >
> > > >> I agree that PRs should not be merged without a chance for others to
> > > >> review. I don't agree to force push and altering the commit tree
> > unless
> > > it
> > > >> is absolutely needed, as it affects everyone. This case doesn't
> > warrant
> > > >> this step, one scenario where a force push might be needed is if
> > > somebody
> > > >> pushed some copyrighted code.
> > > >>
> > > >> Thanks
> > > >>
> > > >> On Thu, Apr 27, 2017 at 6:44 PM, Vlad Rozov <
> v.ro...@datatorrent.com>
> > > >> wrote:
> > > >>
> > > >> I am open to both approaches - two commits or a join commit. Both
> have
> > > >>> pros and cons that we may discuss. What I am strongly against are
> PRs
> > > >>> that
> > > >>> are merged without a chance for other contributors/committers to
> > > review.
> > > >>> There should be a way to forcibly undo such commits.
> > > >>>
> > > >>> Thank you,
> > > >>>
> > > >>> Vlad
> > > >>>
> > > >>>
> > > >>> On 4/27/17 12:41, Pramod Immaneni wrote:
> > > >>>
> > > >>> My comments inline..
> > > 
> > >  On Thu, Apr 27, 2017 at 12:01 PM, Thomas Weise 
> > > wrote:
> > > 
> > >  I'm -1 on using the author field like this in Apex for the reason
> > > stated
> > > 
> > > > (it is also odd to see something like this showing up without
> prior
> > > > discussion).
> > > >
> > > >
> > > > I am not set on this for future commits but would like to say, do
> > we
> > > >
> > >  really
> > >  verify the author field and treat it with importance. For
> example, I
> > >  don't
> > >  think we ever check if the author is the person they say they are,
> > > check
> > >  name, email etc. If I were to use slightly different variations of
> > my
> > >  name
> > >  or email (not that I would do that) would reviewers really verify
> > > that.
> > >  I
> > >  also have checked that tools don't fail on reading a commit
> because
> > >  author
> > >  needs to be in a certain format. I guess contribution stats are
> the
> > > ones
> > >  that will be affected but if used rarely I dont see that being a
> big
> > >  problem. I can understand if we wanted to have strict requirements
> > for
> > >  the
> > >  committer field.
> > > 
> > >  Thanks
> > > 
> > > 
> > >  Consider adding the additional author information to the commit
> > > message.
> > > 
> > > > Thomas
> > > >
> > > > On Thu, Apr 27, 2017 at 11:55 AM, Pramod Immaneni <
> > > > pra...@datatorrent.com>
> > > > wrote:
> > > >
> > > > Agreed it is not regular and should only be used in special
> > > > circumstances.
> > > >
> > > > One example of this is pair programming. It has been done before
> in
> > > >> other
> > > >> projects and searching on google or stackoverflow you can see
> how
> > > >> 

Re: PR merge policy

2017-04-27 Thread Munagala Ramanath
Everybody seems agreed on what the committers should do -- that waiting a
day or two for
others to have a chance to comment seems like an entirely reasonable thing.

The disagreement is about what to do when that policy is violated.

Ram

On Thu, Apr 27, 2017 at 8:37 PM, Thomas Weise  wrote:

> Forced push should not be necessary if committers follow the development
> process.
>
> So why not focus on what the committer should do? Development process and
> guidelines are there for a reason, and the issue was raised before.
>
> In this specific case, there is a string of commits related to the plugin
> feature that IMO should be part of the original review. There wasn't any
> need to rush this, the change wasn't important for the release.
>
> Thomas
>
>
> On Thu, Apr 27, 2017 at 8:11 PM, Munagala Ramanath 
> wrote:
>
> > I agree with Pramod that force pushing should be a rare event done only
> > when there is an immediate
> > need to undo something serious. Doing it just for a policy violation
> should
> > itself be codified in our
> > policies as a policy violation.
> >
> > Why not just commit an improvement on top ?
> >
> > Ram
> >
> > On Thu, Apr 27, 2017 at 7:55 PM, Vlad Rozov 
> > wrote:
> >
> > > Violation of the PR merge policy is a sufficient reason to forcibly
> undo
> > > the commit and such violations affect everyone in the community.
> > >
> > > Thank you,
> > >
> > > Vlad
> > >
> > > On 4/27/17 19:36, Pramod Immaneni wrote:
> > >
> > >> I agree that PRs should not be merged without a chance for others to
> > >> review. I don't agree to force push and altering the commit tree
> unless
> > it
> > >> is absolutely needed, as it affects everyone. This case doesn't
> warrant
> > >> this step, one scenario where a force push might be needed is if
> > somebody
> > >> pushed some copyrighted code.
> > >>
> > >> Thanks
> > >>
> > >> On Thu, Apr 27, 2017 at 6:44 PM, Vlad Rozov 
> > >> wrote:
> > >>
> > >> I am open to both approaches - two commits or a join commit. Both have
> > >>> pros and cons that we may discuss. What I am strongly against are PRs
> > >>> that
> > >>> are merged without a chance for other contributors/committers to
> > review.
> > >>> There should be a way to forcibly undo such commits.
> > >>>
> > >>> Thank you,
> > >>>
> > >>> Vlad
> > >>>
> > >>>
> > >>> On 4/27/17 12:41, Pramod Immaneni wrote:
> > >>>
> > >>> My comments inline..
> > 
> >  On Thu, Apr 27, 2017 at 12:01 PM, Thomas Weise 
> > wrote:
> > 
> >  I'm -1 on using the author field like this in Apex for the reason
> > stated
> > 
> > > (it is also odd to see something like this showing up without prior
> > > discussion).
> > >
> > >
> > > I am not set on this for future commits but would like to say, do
> we
> > >
> >  really
> >  verify the author field and treat it with importance. For example, I
> >  don't
> >  think we ever check if the author is the person they say they are,
> > check
> >  name, email etc. If I were to use slightly different variations of
> my
> >  name
> >  or email (not that I would do that) would reviewers really verify
> > that.
> >  I
> >  also have checked that tools don't fail on reading a commit because
> >  author
> >  needs to be in a certain format. I guess contribution stats are the
> > ones
> >  that will be affected but if used rarely I dont see that being a big
> >  problem. I can understand if we wanted to have strict requirements
> for
> >  the
> >  committer field.
> > 
> >  Thanks
> > 
> > 
> >  Consider adding the additional author information to the commit
> > message.
> > 
> > > Thomas
> > >
> > > On Thu, Apr 27, 2017 at 11:55 AM, Pramod Immaneni <
> > > pra...@datatorrent.com>
> > > wrote:
> > >
> > > Agreed it is not regular and should only be used in special
> > > circumstances.
> > >
> > > One example of this is pair programming. It has been done before in
> > >> other
> > >> projects and searching on google or stackoverflow you can see how
> > >> other
> > >> people have tried to handle it
> > >>
> > >> https://bugs.eclipse.org/bugs/show_bug.cgi?id=375536
> > >> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=451880
> > >> http://stackoverflow.com/questions/7442112/attributing-
> > >> a-single-commit-to-multiple-developers
> > >>
> > >> Thanks
> > >>
> > >> On Thu, Apr 27, 2017 at 9:57 AM, Thomas Weise 
> > wrote:
> > >>
> > >> commit 9856080ede62a4529d730bcb6724c757f5010990
> > >>
> > >>> Author: Pramod Immaneni & Vlad Rozov  > com
> > >>> >
> > >>> Date:   Tue Apr 18 09:37:22 2017 -0700
> > >>>
> > >>> Please don't use the author field in such a way, it leads to
> > 

Re: PR merge policy

2017-04-27 Thread Thomas Weise
Forced push should not be necessary if committers follow the development
process.

So why not focus on what the committer should do? Development process and
guidelines are there for a reason, and the issue was raised before.

In this specific case, there is a string of commits related to the plugin
feature that IMO should be part of the original review. There wasn't any
need to rush this, the change wasn't important for the release.

Thomas


On Thu, Apr 27, 2017 at 8:11 PM, Munagala Ramanath 
wrote:

> I agree with Pramod that force pushing should be a rare event done only
> when there is an immediate
> need to undo something serious. Doing it just for a policy violation should
> itself be codified in our
> policies as a policy violation.
>
> Why not just commit an improvement on top ?
>
> Ram
>
> On Thu, Apr 27, 2017 at 7:55 PM, Vlad Rozov 
> wrote:
>
> > Violation of the PR merge policy is a sufficient reason to forcibly undo
> > the commit and such violations affect everyone in the community.
> >
> > Thank you,
> >
> > Vlad
> >
> > On 4/27/17 19:36, Pramod Immaneni wrote:
> >
> >> I agree that PRs should not be merged without a chance for others to
> >> review. I don't agree to force push and altering the commit tree unless
> it
> >> is absolutely needed, as it affects everyone. This case doesn't warrant
> >> this step, one scenario where a force push might be needed is if
> somebody
> >> pushed some copyrighted code.
> >>
> >> Thanks
> >>
> >> On Thu, Apr 27, 2017 at 6:44 PM, Vlad Rozov 
> >> wrote:
> >>
> >> I am open to both approaches - two commits or a join commit. Both have
> >>> pros and cons that we may discuss. What I am strongly against are PRs
> >>> that
> >>> are merged without a chance for other contributors/committers to
> review.
> >>> There should be a way to forcibly undo such commits.
> >>>
> >>> Thank you,
> >>>
> >>> Vlad
> >>>
> >>>
> >>> On 4/27/17 12:41, Pramod Immaneni wrote:
> >>>
> >>> My comments inline..
> 
>  On Thu, Apr 27, 2017 at 12:01 PM, Thomas Weise 
> wrote:
> 
>  I'm -1 on using the author field like this in Apex for the reason
> stated
> 
> > (it is also odd to see something like this showing up without prior
> > discussion).
> >
> >
> > I am not set on this for future commits but would like to say, do we
> >
>  really
>  verify the author field and treat it with importance. For example, I
>  don't
>  think we ever check if the author is the person they say they are,
> check
>  name, email etc. If I were to use slightly different variations of my
>  name
>  or email (not that I would do that) would reviewers really verify
> that.
>  I
>  also have checked that tools don't fail on reading a commit because
>  author
>  needs to be in a certain format. I guess contribution stats are the
> ones
>  that will be affected but if used rarely I dont see that being a big
>  problem. I can understand if we wanted to have strict requirements for
>  the
>  committer field.
> 
>  Thanks
> 
> 
>  Consider adding the additional author information to the commit
> message.
> 
> > Thomas
> >
> > On Thu, Apr 27, 2017 at 11:55 AM, Pramod Immaneni <
> > pra...@datatorrent.com>
> > wrote:
> >
> > Agreed it is not regular and should only be used in special
> > circumstances.
> >
> > One example of this is pair programming. It has been done before in
> >> other
> >> projects and searching on google or stackoverflow you can see how
> >> other
> >> people have tried to handle it
> >>
> >> https://bugs.eclipse.org/bugs/show_bug.cgi?id=375536
> >> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=451880
> >> http://stackoverflow.com/questions/7442112/attributing-
> >> a-single-commit-to-multiple-developers
> >>
> >> Thanks
> >>
> >> On Thu, Apr 27, 2017 at 9:57 AM, Thomas Weise 
> wrote:
> >>
> >> commit 9856080ede62a4529d730bcb6724c757f5010990
> >>
> >>> Author: Pramod Immaneni & Vlad Rozov  com
> >>> >
> >>> Date:   Tue Apr 18 09:37:22 2017 -0700
> >>>
> >>> Please don't use the author field in such a way, it leads to
> >>> incorrect
> >>> tracking of contributions.
> >>>
> >>> Either have separate commits or have one author.
> >>>
> >>> Thanks
> >>>
> >>>
> >>>
> >>> On Thu, Apr 27, 2017 at 9:31 AM, Pramod Immaneni <
> >>>
> >>> pra...@datatorrent.com
> >> wrote:
> >>
> >>> The issue was two different plugin models were developed, one for
> >>>
>  pre-launch and other for post-launch. I felt that the one built
> 
>  latter
> >>>
> >> was
> >>
> >>> better and it would be better to have a uniform interface 

Re: PR merge policy

2017-04-27 Thread Munagala Ramanath
I agree with Pramod that force pushing should be a rare event done only
when there is an immediate
need to undo something serious. Doing it just for a policy violation should
itself be codified in our
policies as a policy violation.

Why not just commit an improvement on top ?

Ram

On Thu, Apr 27, 2017 at 7:55 PM, Vlad Rozov  wrote:

> Violation of the PR merge policy is a sufficient reason to forcibly undo
> the commit and such violations affect everyone in the community.
>
> Thank you,
>
> Vlad
>
> On 4/27/17 19:36, Pramod Immaneni wrote:
>
>> I agree that PRs should not be merged without a chance for others to
>> review. I don't agree to force push and altering the commit tree unless it
>> is absolutely needed, as it affects everyone. This case doesn't warrant
>> this step, one scenario where a force push might be needed is if somebody
>> pushed some copyrighted code.
>>
>> Thanks
>>
>> On Thu, Apr 27, 2017 at 6:44 PM, Vlad Rozov 
>> wrote:
>>
>> I am open to both approaches - two commits or a join commit. Both have
>>> pros and cons that we may discuss. What I am strongly against are PRs
>>> that
>>> are merged without a chance for other contributors/committers to review.
>>> There should be a way to forcibly undo such commits.
>>>
>>> Thank you,
>>>
>>> Vlad
>>>
>>>
>>> On 4/27/17 12:41, Pramod Immaneni wrote:
>>>
>>> My comments inline..

 On Thu, Apr 27, 2017 at 12:01 PM, Thomas Weise  wrote:

 I'm -1 on using the author field like this in Apex for the reason stated

> (it is also odd to see something like this showing up without prior
> discussion).
>
>
> I am not set on this for future commits but would like to say, do we
>
 really
 verify the author field and treat it with importance. For example, I
 don't
 think we ever check if the author is the person they say they are, check
 name, email etc. If I were to use slightly different variations of my
 name
 or email (not that I would do that) would reviewers really verify that.
 I
 also have checked that tools don't fail on reading a commit because
 author
 needs to be in a certain format. I guess contribution stats are the ones
 that will be affected but if used rarely I dont see that being a big
 problem. I can understand if we wanted to have strict requirements for
 the
 committer field.

 Thanks


 Consider adding the additional author information to the commit message.

> Thomas
>
> On Thu, Apr 27, 2017 at 11:55 AM, Pramod Immaneni <
> pra...@datatorrent.com>
> wrote:
>
> Agreed it is not regular and should only be used in special
> circumstances.
>
> One example of this is pair programming. It has been done before in
>> other
>> projects and searching on google or stackoverflow you can see how
>> other
>> people have tried to handle it
>>
>> https://bugs.eclipse.org/bugs/show_bug.cgi?id=375536
>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=451880
>> http://stackoverflow.com/questions/7442112/attributing-
>> a-single-commit-to-multiple-developers
>>
>> Thanks
>>
>> On Thu, Apr 27, 2017 at 9:57 AM, Thomas Weise  wrote:
>>
>> commit 9856080ede62a4529d730bcb6724c757f5010990
>>
>>> Author: Pramod Immaneni & Vlad Rozov >> >
>>> Date:   Tue Apr 18 09:37:22 2017 -0700
>>>
>>> Please don't use the author field in such a way, it leads to
>>> incorrect
>>> tracking of contributions.
>>>
>>> Either have separate commits or have one author.
>>>
>>> Thanks
>>>
>>>
>>>
>>> On Thu, Apr 27, 2017 at 9:31 AM, Pramod Immaneni <
>>>
>>> pra...@datatorrent.com
>> wrote:
>>
>>> The issue was two different plugin models were developed, one for
>>>
 pre-launch and other for post-launch. I felt that the one built

 latter
>>>
>> was
>>
>>> better and it would be better to have a uniform interface for the

 users
>>>
>> and
>>
>>> hence asked for the changes.

 On Thu, Apr 27, 2017 at 9:05 AM, Thomas Weise 

 wrote:
>>>
>> I think the plugins feature could have benefited from better
>>
>>> original

>>> review, which would have eliminated much of the back and forth
>>
>>> after

>>> the
>>
>>> fact.

>
> On Thu, Apr 27, 2017 at 8:20 AM, Vlad Rozov <
>
> v.ro...@datatorrent.com

>>> wrote:
>>
>>> Pramod,
>
>> No, it is not a request to update the apex.apache.org (to do
>>
>> that
>
 we
>>
>> need
>>>
 to file JIRA). It is a reminder that 

Re: PR merge policy

2017-04-27 Thread Vlad Rozov
Violation of the PR merge policy is a sufficient reason to forcibly undo 
the commit and such violations affect everyone in the community.


Thank you,

Vlad

On 4/27/17 19:36, Pramod Immaneni wrote:

I agree that PRs should not be merged without a chance for others to
review. I don't agree to force push and altering the commit tree unless it
is absolutely needed, as it affects everyone. This case doesn't warrant
this step, one scenario where a force push might be needed is if somebody
pushed some copyrighted code.

Thanks

On Thu, Apr 27, 2017 at 6:44 PM, Vlad Rozov  wrote:


I am open to both approaches - two commits or a join commit. Both have
pros and cons that we may discuss. What I am strongly against are PRs that
are merged without a chance for other contributors/committers to review.
There should be a way to forcibly undo such commits.

Thank you,

Vlad


On 4/27/17 12:41, Pramod Immaneni wrote:


My comments inline..

On Thu, Apr 27, 2017 at 12:01 PM, Thomas Weise  wrote:

I'm -1 on using the author field like this in Apex for the reason stated

(it is also odd to see something like this showing up without prior
discussion).


I am not set on this for future commits but would like to say, do we

really
verify the author field and treat it with importance. For example, I don't
think we ever check if the author is the person they say they are, check
name, email etc. If I were to use slightly different variations of my name
or email (not that I would do that) would reviewers really verify that. I
also have checked that tools don't fail on reading a commit because author
needs to be in a certain format. I guess contribution stats are the ones
that will be affected but if used rarely I dont see that being a big
problem. I can understand if we wanted to have strict requirements for the
committer field.

Thanks


Consider adding the additional author information to the commit message.

Thomas

On Thu, Apr 27, 2017 at 11:55 AM, Pramod Immaneni <
pra...@datatorrent.com>
wrote:

Agreed it is not regular and should only be used in special
circumstances.


One example of this is pair programming. It has been done before in
other
projects and searching on google or stackoverflow you can see how other
people have tried to handle it

https://bugs.eclipse.org/bugs/show_bug.cgi?id=375536
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=451880
http://stackoverflow.com/questions/7442112/attributing-
a-single-commit-to-multiple-developers

Thanks

On Thu, Apr 27, 2017 at 9:57 AM, Thomas Weise  wrote:

commit 9856080ede62a4529d730bcb6724c757f5010990

Author: Pramod Immaneni & Vlad Rozov 
Date:   Tue Apr 18 09:37:22 2017 -0700

Please don't use the author field in such a way, it leads to incorrect
tracking of contributions.

Either have separate commits or have one author.

Thanks



On Thu, Apr 27, 2017 at 9:31 AM, Pramod Immaneni <


pra...@datatorrent.com
wrote:

The issue was two different plugin models were developed, one for

pre-launch and other for post-launch. I felt that the one built


latter

was

better and it would be better to have a uniform interface for the


users

and

hence asked for the changes.

On Thu, Apr 27, 2017 at 9:05 AM, Thomas Weise 


wrote:

I think the plugins feature could have benefited from better

original

review, which would have eliminated much of the back and forth

after

the

fact.


On Thu, Apr 27, 2017 at 8:20 AM, Vlad Rozov <


v.ro...@datatorrent.com

wrote:

Pramod,

No, it is not a request to update the apex.apache.org (to do


that

we


need

to file JIRA). It is a reminder that it is against Apex policy to


merge

PR

without giving enough time for others to review it (few hours


after

PR

was

open).

Thank you,

Vlad

On 4/27/17 08:05, Pramod Immaneni wrote:

Vlad, are you asking for a consensus on the policy to make it
official

(publish on website etc). I believe we have one. However, I did

not

see

much difference between what you said on Mar 26th to what I

proposed

on

Mar

24th. Is the main difference any committer can merge (not just


the

main

reviewer) as long as there are no objections from others. In

that

case,

I


am fine with it.

On Thu, Apr 27, 2017 at 7:44 AM, Vlad Rozov <


v.ro...@datatorrent.com>

wrote:

This is a friendly reminder regarding PR merge policy.


Thank you,

Vlad


On 3/23/17 12:58, Vlad Rozov wrote:

Lately there were few instances where PR open against apex-core


and

apex-malhar were merged just few hours after it being open and

JIRA

being

raised without giving chance for other contributors to review


and

comment.

I'd suggest that we stop such practice no matter how trivial


those

changes

are. This equally applies to documentation. In a rear cases


where

PR


is

urgent (for example one that fixes compilation error), I'd

suggest

that

a

committer who plans to merge the PR sends an explicit



Re: PR merge policy

2017-04-27 Thread Pramod Immaneni
I agree that PRs should not be merged without a chance for others to
review. I don't agree to force push and altering the commit tree unless it
is absolutely needed, as it affects everyone. This case doesn't warrant
this step, one scenario where a force push might be needed is if somebody
pushed some copyrighted code.

Thanks

On Thu, Apr 27, 2017 at 6:44 PM, Vlad Rozov  wrote:

> I am open to both approaches - two commits or a join commit. Both have
> pros and cons that we may discuss. What I am strongly against are PRs that
> are merged without a chance for other contributors/committers to review.
> There should be a way to forcibly undo such commits.
>
> Thank you,
>
> Vlad
>
>
> On 4/27/17 12:41, Pramod Immaneni wrote:
>
>> My comments inline..
>>
>> On Thu, Apr 27, 2017 at 12:01 PM, Thomas Weise  wrote:
>>
>> I'm -1 on using the author field like this in Apex for the reason stated
>>> (it is also odd to see something like this showing up without prior
>>> discussion).
>>>
>>>
>>> I am not set on this for future commits but would like to say, do we
>> really
>> verify the author field and treat it with importance. For example, I don't
>> think we ever check if the author is the person they say they are, check
>> name, email etc. If I were to use slightly different variations of my name
>> or email (not that I would do that) would reviewers really verify that. I
>> also have checked that tools don't fail on reading a commit because author
>> needs to be in a certain format. I guess contribution stats are the ones
>> that will be affected but if used rarely I dont see that being a big
>> problem. I can understand if we wanted to have strict requirements for the
>> committer field.
>>
>> Thanks
>>
>>
>> Consider adding the additional author information to the commit message.
>>>
>>> Thomas
>>>
>>> On Thu, Apr 27, 2017 at 11:55 AM, Pramod Immaneni <
>>> pra...@datatorrent.com>
>>> wrote:
>>>
>>> Agreed it is not regular and should only be used in special

>>> circumstances.
>>>
 One example of this is pair programming. It has been done before in
 other
 projects and searching on google or stackoverflow you can see how other
 people have tried to handle it

 https://bugs.eclipse.org/bugs/show_bug.cgi?id=375536
 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=451880
 http://stackoverflow.com/questions/7442112/attributing-
 a-single-commit-to-multiple-developers

 Thanks

 On Thu, Apr 27, 2017 at 9:57 AM, Thomas Weise  wrote:

 commit 9856080ede62a4529d730bcb6724c757f5010990
> Author: Pramod Immaneni & Vlad Rozov 
> Date:   Tue Apr 18 09:37:22 2017 -0700
>
> Please don't use the author field in such a way, it leads to incorrect
> tracking of contributions.
>
> Either have separate commits or have one author.
>
> Thanks
>
>
>
> On Thu, Apr 27, 2017 at 9:31 AM, Pramod Immaneni <
>
 pra...@datatorrent.com
>>>
 wrote:
>
> The issue was two different plugin models were developed, one for
>> pre-launch and other for post-launch. I felt that the one built
>>
> latter
>>>
 was
>
>> better and it would be better to have a uniform interface for the
>>
> users
>>>
 and
>
>> hence asked for the changes.
>>
>> On Thu, Apr 27, 2017 at 9:05 AM, Thomas Weise 
>>
> wrote:
>>>
 I think the plugins feature could have benefited from better
>>>
>> original
>>>
 review, which would have eliminated much of the back and forth
>>>
>> after
>>>
 the
>
>> fact.
>>>
>>>
>>> On Thu, Apr 27, 2017 at 8:20 AM, Vlad Rozov <
>>>
>> v.ro...@datatorrent.com
>>>
 wrote:
>>>
>>> Pramod,

 No, it is not a request to update the apex.apache.org (to do

>>> that
>>>
 we

> need
>>>
 to file JIRA). It is a reminder that it is against Apex policy to

>>> merge
>
>> PR
>>>
 without giving enough time for others to review it (few hours

>>> after
>>>
 PR
>
>> was
>>>
 open).

 Thank you,

 Vlad

 On 4/27/17 08:05, Pramod Immaneni wrote:

 Vlad, are you asking for a consensus on the policy to make it
>
 official
>
>> (publish on website etc). I believe we have one. However, I did
>
 not

> see
>>
>>> much difference between what you said on Mar 26th to what I
>
 proposed

> on
>>
>>> Mar
> 24th. Is the main difference any committer can merge (not just
>
 the
>>>
 main
>>
>>> reviewer) as long as there are no objections from others. In
>
 that
>>>
 case,
>>

Re: PR merge policy

2017-04-27 Thread Vlad Rozov
I am open to both approaches - two commits or a join commit. Both have 
pros and cons that we may discuss. What I am strongly against are PRs 
that are merged without a chance for other contributors/committers to 
review. There should be a way to forcibly undo such commits.


Thank you,

Vlad

On 4/27/17 12:41, Pramod Immaneni wrote:

My comments inline..

On Thu, Apr 27, 2017 at 12:01 PM, Thomas Weise  wrote:


I'm -1 on using the author field like this in Apex for the reason stated
(it is also odd to see something like this showing up without prior
discussion).



I am not set on this for future commits but would like to say, do we really
verify the author field and treat it with importance. For example, I don't
think we ever check if the author is the person they say they are, check
name, email etc. If I were to use slightly different variations of my name
or email (not that I would do that) would reviewers really verify that. I
also have checked that tools don't fail on reading a commit because author
needs to be in a certain format. I guess contribution stats are the ones
that will be affected but if used rarely I dont see that being a big
problem. I can understand if we wanted to have strict requirements for the
committer field.

Thanks



Consider adding the additional author information to the commit message.

Thomas

On Thu, Apr 27, 2017 at 11:55 AM, Pramod Immaneni 
wrote:


Agreed it is not regular and should only be used in special

circumstances.

One example of this is pair programming. It has been done before in other
projects and searching on google or stackoverflow you can see how other
people have tried to handle it

https://bugs.eclipse.org/bugs/show_bug.cgi?id=375536
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=451880
http://stackoverflow.com/questions/7442112/attributing-
a-single-commit-to-multiple-developers

Thanks

On Thu, Apr 27, 2017 at 9:57 AM, Thomas Weise  wrote:


commit 9856080ede62a4529d730bcb6724c757f5010990
Author: Pramod Immaneni & Vlad Rozov 
Date:   Tue Apr 18 09:37:22 2017 -0700

Please don't use the author field in such a way, it leads to incorrect
tracking of contributions.

Either have separate commits or have one author.

Thanks



On Thu, Apr 27, 2017 at 9:31 AM, Pramod Immaneni <

pra...@datatorrent.com

wrote:


The issue was two different plugin models were developed, one for
pre-launch and other for post-launch. I felt that the one built

latter

was

better and it would be better to have a uniform interface for the

users

and

hence asked for the changes.

On Thu, Apr 27, 2017 at 9:05 AM, Thomas Weise 

wrote:

I think the plugins feature could have benefited from better

original

review, which would have eliminated much of the back and forth

after

the

fact.


On Thu, Apr 27, 2017 at 8:20 AM, Vlad Rozov <

v.ro...@datatorrent.com

wrote:


Pramod,

No, it is not a request to update the apex.apache.org (to do

that

we

need

to file JIRA). It is a reminder that it is against Apex policy to

merge

PR

without giving enough time for others to review it (few hours

after

PR

was

open).

Thank you,

Vlad

On 4/27/17 08:05, Pramod Immaneni wrote:


Vlad, are you asking for a consensus on the policy to make it

official

(publish on website etc). I believe we have one. However, I did

not

see

much difference between what you said on Mar 26th to what I

proposed

on

Mar
24th. Is the main difference any committer can merge (not just

the

main

reviewer) as long as there are no objections from others. In

that

case,

I

am fine with it.

On Thu, Apr 27, 2017 at 7:44 AM, Vlad Rozov <

v.ro...@datatorrent.com>

wrote:

This is a friendly reminder regarding PR merge policy.

Thank you,

Vlad


On 3/23/17 12:58, Vlad Rozov wrote:

Lately there were few instances where PR open against apex-core

and

apex-malhar were merged just few hours after it being open and

JIRA

being
raised without giving chance for other contributors to review

and

comment.
I'd suggest that we stop such practice no matter how trivial

those

changes
are. This equally applies to documentation. In a rear cases

where

PR

is

urgent (for example one that fixes compilation error), I'd

suggest

that

a
committer who plans to merge the PR sends an explicit

notification

to

dev@apex and gives others a reasonable time to respond.

Thank you,

Vlad







[GitHub] apex-core pull request #520: APEXCORE-711 create a new attribute CUSTOM_SSL_...

2017-04-27 Thread sanjaypujare
GitHub user sanjaypujare opened a pull request:

https://github.com/apache/apex-core/pull/520

APEXCORE-711 create a new attribute CUSTOM_SSL_SERVER_CONFIG and use its 
value to set custom ssl server config

@PramodSSImmaneni  pls review and merge as appropriate

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/sanjaypujare/apex-core 
APEXCORE-711.sanjay.master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/apex-core/pull/520.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #520


commit 660005fc77ebd6f7b6e0c23d6e82f9a08a9527d4
Author: Sanjay Pujare 
Date:   2017-04-27T21:56:17Z

APEXCORE-711 create a new attribute CUSTOM_SSL_SERVER_CONFIG and use its 
value to set custom ssl server config




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Commented] (APEXCORE-711) Support custom SSL keystore for the Stram REST API web service

2017-04-27 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/APEXCORE-711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15987779#comment-15987779
 ] 

ASF GitHub Bot commented on APEXCORE-711:
-

GitHub user sanjaypujare opened a pull request:

https://github.com/apache/apex-core/pull/520

APEXCORE-711 create a new attribute CUSTOM_SSL_SERVER_CONFIG and use its 
value to set custom ssl server config

@PramodSSImmaneni  pls review and merge as appropriate

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/sanjaypujare/apex-core 
APEXCORE-711.sanjay.master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/apex-core/pull/520.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #520


commit 660005fc77ebd6f7b6e0c23d6e82f9a08a9527d4
Author: Sanjay Pujare 
Date:   2017-04-27T21:56:17Z

APEXCORE-711 create a new attribute CUSTOM_SSL_SERVER_CONFIG and use its 
value to set custom ssl server config




> Support custom SSL keystore for the Stram REST API web service
> --
>
> Key: APEXCORE-711
> URL: https://issues.apache.org/jira/browse/APEXCORE-711
> Project: Apache Apex Core
>  Issue Type: Improvement
>Reporter: Sanjay M Pujare
>Assignee: Sanjay M Pujare
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> Currently StrAM supports only the default Hadoop SSL configuration for the 
> web-service because it uses org.apache.hadoop.yarn.webapp.WebApps helper 
> class which has the limitation of only using the default Hadoop SSL config 
> that is read from Hadoop's ssl-server.xml resource file. Some users have run 
> into a situation where Hadoops' SSL keystore is not available on most cluster 
> nodes or the Stram process doesn't have read access to the keystore even when 
> present. So there is a need for the Stram to use a custom SSL keystore and 
> configuration that does not suffer from these limitations.
> There is already a PR https://github.com/apache/hadoop/pull/213 to Hadoop to 
> support this in Hadoop and it is in the process of getting merged soon.
> After that Stram needs to be enhanced (this JIRA) to accept the location of a 
> custom ssl-server.xml file (supplied by the client via a DAG attribute) and 
> use the values from that file to set up the config object to be passed to 
> WebApps which will end up using the custom SSL configuration. This approach 
> has already been verified in a prototype.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


Re: PR merge policy

2017-04-27 Thread Thomas Weise
I'm -1 on using the author field like this in Apex for the reason stated
(it is also odd to see something like this showing up without prior
discussion).

Consider adding the additional author information to the commit message.

Thomas

On Thu, Apr 27, 2017 at 11:55 AM, Pramod Immaneni 
wrote:

> Agreed it is not regular and should only be used in special circumstances.
> One example of this is pair programming. It has been done before in other
> projects and searching on google or stackoverflow you can see how other
> people have tried to handle it
>
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=375536
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=451880
> http://stackoverflow.com/questions/7442112/attributing-
> a-single-commit-to-multiple-developers
>
> Thanks
>
> On Thu, Apr 27, 2017 at 9:57 AM, Thomas Weise  wrote:
>
> > commit 9856080ede62a4529d730bcb6724c757f5010990
> > Author: Pramod Immaneni & Vlad Rozov 
> > Date:   Tue Apr 18 09:37:22 2017 -0700
> >
> > Please don't use the author field in such a way, it leads to incorrect
> > tracking of contributions.
> >
> > Either have separate commits or have one author.
> >
> > Thanks
> >
> >
> >
> > On Thu, Apr 27, 2017 at 9:31 AM, Pramod Immaneni  >
> > wrote:
> >
> > > The issue was two different plugin models were developed, one for
> > > pre-launch and other for post-launch. I felt that the one built latter
> > was
> > > better and it would be better to have a uniform interface for the users
> > and
> > > hence asked for the changes.
> > >
> > > On Thu, Apr 27, 2017 at 9:05 AM, Thomas Weise  wrote:
> > >
> > > > I think the plugins feature could have benefited from better original
> > > > review, which would have eliminated much of the back and forth after
> > the
> > > > fact.
> > > >
> > > >
> > > > On Thu, Apr 27, 2017 at 8:20 AM, Vlad Rozov  >
> > > > wrote:
> > > >
> > > > > Pramod,
> > > > >
> > > > > No, it is not a request to update the apex.apache.org (to do that
> we
> > > > need
> > > > > to file JIRA). It is a reminder that it is against Apex policy to
> > merge
> > > > PR
> > > > > without giving enough time for others to review it (few hours after
> > PR
> > > > was
> > > > > open).
> > > > >
> > > > > Thank you,
> > > > >
> > > > > Vlad
> > > > >
> > > > > On 4/27/17 08:05, Pramod Immaneni wrote:
> > > > >
> > > > >> Vlad, are you asking for a consensus on the policy to make it
> > official
> > > > >> (publish on website etc). I believe we have one. However, I did
> not
> > > see
> > > > >> much difference between what you said on Mar 26th to what I
> proposed
> > > on
> > > > >> Mar
> > > > >> 24th. Is the main difference any committer can merge (not just the
> > > main
> > > > >> reviewer) as long as there are no objections from others. In that
> > > case,
> > > > I
> > > > >> am fine with it.
> > > > >>
> > > > >> On Thu, Apr 27, 2017 at 7:44 AM, Vlad Rozov <
> > v.ro...@datatorrent.com>
> > > > >> wrote:
> > > > >>
> > > > >> This is a friendly reminder regarding PR merge policy.
> > > > >>>
> > > > >>> Thank you,
> > > > >>>
> > > > >>> Vlad
> > > > >>>
> > > > >>>
> > > > >>> On 3/23/17 12:58, Vlad Rozov wrote:
> > > > >>>
> > > > >>> Lately there were few instances where PR open against apex-core
> and
> > > >  apex-malhar were merged just few hours after it being open and
> > JIRA
> > > >  being
> > > >  raised without giving chance for other contributors to review
> and
> > > >  comment.
> > > >  I'd suggest that we stop such practice no matter how trivial
> those
> > > >  changes
> > > >  are. This equally applies to documentation. In a rear cases
> where
> > PR
> > > > is
> > > >  urgent (for example one that fixes compilation error), I'd
> suggest
> > > > that
> > > >  a
> > > >  committer who plans to merge the PR sends an explicit
> notification
> > > to
> > > >  dev@apex and gives others a reasonable time to respond.
> > > > 
> > > >  Thank you,
> > > > 
> > > >  Vlad
> > > > 
> > > > 
> > > > 
> > > > >
> > > >
> > >
> >
>


Re: PR merge policy

2017-04-27 Thread Pramod Immaneni
Agreed it is not regular and should only be used in special circumstances.
One example of this is pair programming. It has been done before in other
projects and searching on google or stackoverflow you can see how other
people have tried to handle it

https://bugs.eclipse.org/bugs/show_bug.cgi?id=375536
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=451880
http://stackoverflow.com/questions/7442112/attributing-a-single-commit-to-multiple-developers

Thanks

On Thu, Apr 27, 2017 at 9:57 AM, Thomas Weise  wrote:

> commit 9856080ede62a4529d730bcb6724c757f5010990
> Author: Pramod Immaneni & Vlad Rozov 
> Date:   Tue Apr 18 09:37:22 2017 -0700
>
> Please don't use the author field in such a way, it leads to incorrect
> tracking of contributions.
>
> Either have separate commits or have one author.
>
> Thanks
>
>
>
> On Thu, Apr 27, 2017 at 9:31 AM, Pramod Immaneni 
> wrote:
>
> > The issue was two different plugin models were developed, one for
> > pre-launch and other for post-launch. I felt that the one built latter
> was
> > better and it would be better to have a uniform interface for the users
> and
> > hence asked for the changes.
> >
> > On Thu, Apr 27, 2017 at 9:05 AM, Thomas Weise  wrote:
> >
> > > I think the plugins feature could have benefited from better original
> > > review, which would have eliminated much of the back and forth after
> the
> > > fact.
> > >
> > >
> > > On Thu, Apr 27, 2017 at 8:20 AM, Vlad Rozov 
> > > wrote:
> > >
> > > > Pramod,
> > > >
> > > > No, it is not a request to update the apex.apache.org (to do that we
> > > need
> > > > to file JIRA). It is a reminder that it is against Apex policy to
> merge
> > > PR
> > > > without giving enough time for others to review it (few hours after
> PR
> > > was
> > > > open).
> > > >
> > > > Thank you,
> > > >
> > > > Vlad
> > > >
> > > > On 4/27/17 08:05, Pramod Immaneni wrote:
> > > >
> > > >> Vlad, are you asking for a consensus on the policy to make it
> official
> > > >> (publish on website etc). I believe we have one. However, I did not
> > see
> > > >> much difference between what you said on Mar 26th to what I proposed
> > on
> > > >> Mar
> > > >> 24th. Is the main difference any committer can merge (not just the
> > main
> > > >> reviewer) as long as there are no objections from others. In that
> > case,
> > > I
> > > >> am fine with it.
> > > >>
> > > >> On Thu, Apr 27, 2017 at 7:44 AM, Vlad Rozov <
> v.ro...@datatorrent.com>
> > > >> wrote:
> > > >>
> > > >> This is a friendly reminder regarding PR merge policy.
> > > >>>
> > > >>> Thank you,
> > > >>>
> > > >>> Vlad
> > > >>>
> > > >>>
> > > >>> On 3/23/17 12:58, Vlad Rozov wrote:
> > > >>>
> > > >>> Lately there were few instances where PR open against apex-core and
> > >  apex-malhar were merged just few hours after it being open and
> JIRA
> > >  being
> > >  raised without giving chance for other contributors to review and
> > >  comment.
> > >  I'd suggest that we stop such practice no matter how trivial those
> > >  changes
> > >  are. This equally applies to documentation. In a rear cases where
> PR
> > > is
> > >  urgent (for example one that fixes compilation error), I'd suggest
> > > that
> > >  a
> > >  committer who plans to merge the PR sends an explicit notification
> > to
> > >  dev@apex and gives others a reasonable time to respond.
> > > 
> > >  Thank you,
> > > 
> > >  Vlad
> > > 
> > > 
> > > 
> > > >
> > >
> >
>


Re: PR merge policy

2017-04-27 Thread Thomas Weise
commit 9856080ede62a4529d730bcb6724c757f5010990
Author: Pramod Immaneni & Vlad Rozov 
Date:   Tue Apr 18 09:37:22 2017 -0700

Please don't use the author field in such a way, it leads to incorrect
tracking of contributions.

Either have separate commits or have one author.

Thanks



On Thu, Apr 27, 2017 at 9:31 AM, Pramod Immaneni 
wrote:

> The issue was two different plugin models were developed, one for
> pre-launch and other for post-launch. I felt that the one built latter was
> better and it would be better to have a uniform interface for the users and
> hence asked for the changes.
>
> On Thu, Apr 27, 2017 at 9:05 AM, Thomas Weise  wrote:
>
> > I think the plugins feature could have benefited from better original
> > review, which would have eliminated much of the back and forth after the
> > fact.
> >
> >
> > On Thu, Apr 27, 2017 at 8:20 AM, Vlad Rozov 
> > wrote:
> >
> > > Pramod,
> > >
> > > No, it is not a request to update the apex.apache.org (to do that we
> > need
> > > to file JIRA). It is a reminder that it is against Apex policy to merge
> > PR
> > > without giving enough time for others to review it (few hours after PR
> > was
> > > open).
> > >
> > > Thank you,
> > >
> > > Vlad
> > >
> > > On 4/27/17 08:05, Pramod Immaneni wrote:
> > >
> > >> Vlad, are you asking for a consensus on the policy to make it official
> > >> (publish on website etc). I believe we have one. However, I did not
> see
> > >> much difference between what you said on Mar 26th to what I proposed
> on
> > >> Mar
> > >> 24th. Is the main difference any committer can merge (not just the
> main
> > >> reviewer) as long as there are no objections from others. In that
> case,
> > I
> > >> am fine with it.
> > >>
> > >> On Thu, Apr 27, 2017 at 7:44 AM, Vlad Rozov 
> > >> wrote:
> > >>
> > >> This is a friendly reminder regarding PR merge policy.
> > >>>
> > >>> Thank you,
> > >>>
> > >>> Vlad
> > >>>
> > >>>
> > >>> On 3/23/17 12:58, Vlad Rozov wrote:
> > >>>
> > >>> Lately there were few instances where PR open against apex-core and
> >  apex-malhar were merged just few hours after it being open and JIRA
> >  being
> >  raised without giving chance for other contributors to review and
> >  comment.
> >  I'd suggest that we stop such practice no matter how trivial those
> >  changes
> >  are. This equally applies to documentation. In a rear cases where PR
> > is
> >  urgent (for example one that fixes compilation error), I'd suggest
> > that
> >  a
> >  committer who plans to merge the PR sends an explicit notification
> to
> >  dev@apex and gives others a reasonable time to respond.
> > 
> >  Thank you,
> > 
> >  Vlad
> > 
> > 
> > 
> > >
> >
>


Re: PR merge policy

2017-04-27 Thread Vlad Rozov

Pramod,

No, it is not a request to update the apex.apache.org (to do that we 
need to file JIRA). It is a reminder that it is against Apex policy to 
merge PR without giving enough time for others to review it (few hours 
after PR was open).


Thank you,

Vlad
On 4/27/17 08:05, Pramod Immaneni wrote:

Vlad, are you asking for a consensus on the policy to make it official
(publish on website etc). I believe we have one. However, I did not see
much difference between what you said on Mar 26th to what I proposed on Mar
24th. Is the main difference any committer can merge (not just the main
reviewer) as long as there are no objections from others. In that case, I
am fine with it.

On Thu, Apr 27, 2017 at 7:44 AM, Vlad Rozov  wrote:


This is a friendly reminder regarding PR merge policy.

Thank you,

Vlad


On 3/23/17 12:58, Vlad Rozov wrote:


Lately there were few instances where PR open against apex-core and
apex-malhar were merged just few hours after it being open and JIRA being
raised without giving chance for other contributors to review and comment.
I'd suggest that we stop such practice no matter how trivial those changes
are. This equally applies to documentation. In a rear cases where PR is
urgent (for example one that fixes compilation error), I'd suggest that a
committer who plans to merge the PR sends an explicit notification to
dev@apex and gives others a reasonable time to respond.

Thank you,

Vlad






Re: PR merge policy

2017-04-27 Thread Pramod Immaneni
Vlad, are you asking for a consensus on the policy to make it official
(publish on website etc). I believe we have one. However, I did not see
much difference between what you said on Mar 26th to what I proposed on Mar
24th. Is the main difference any committer can merge (not just the main
reviewer) as long as there are no objections from others. In that case, I
am fine with it.

On Thu, Apr 27, 2017 at 7:44 AM, Vlad Rozov  wrote:

> This is a friendly reminder regarding PR merge policy.
>
> Thank you,
>
> Vlad
>
>
> On 3/23/17 12:58, Vlad Rozov wrote:
>
>> Lately there were few instances where PR open against apex-core and
>> apex-malhar were merged just few hours after it being open and JIRA being
>> raised without giving chance for other contributors to review and comment.
>> I'd suggest that we stop such practice no matter how trivial those changes
>> are. This equally applies to documentation. In a rear cases where PR is
>> urgent (for example one that fixes compilation error), I'd suggest that a
>> committer who plans to merge the PR sends an explicit notification to
>> dev@apex and gives others a reasonable time to respond.
>>
>> Thank you,
>>
>> Vlad
>>
>>
>


Re: PR merge policy

2017-04-27 Thread Vlad Rozov

This is a friendly reminder regarding PR merge policy.

Thank you,

Vlad

On 3/23/17 12:58, Vlad Rozov wrote:
Lately there were few instances where PR open against apex-core and 
apex-malhar were merged just few hours after it being open and JIRA 
being raised without giving chance for other contributors to review 
and comment. I'd suggest that we stop such practice no matter how 
trivial those changes are. This equally applies to documentation. In a 
rear cases where PR is urgent (for example one that fixes compilation 
error), I'd suggest that a committer who plans to merge the PR sends 
an explicit notification to dev@apex and gives others a reasonable 
time to respond.


Thank you,

Vlad





[jira] [Commented] (APEXMALHAR-2484) BlockWriter for writing the part files into the specified directory

2017-04-27 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/APEXMALHAR-2484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15986303#comment-15986303
 ] 

ASF GitHub Bot commented on APEXMALHAR-2484:


Github user chaithu14 closed the pull request at:

https://github.com/apache/apex-malhar/pull/611


> BlockWriter for writing the part files into the specified directory
> ---
>
> Key: APEXMALHAR-2484
> URL: https://issues.apache.org/jira/browse/APEXMALHAR-2484
> Project: Apache Apex Malhar
>  Issue Type: Task
>Reporter: Chaitanya
>Assignee: Chaitanya
>
> Use case: Suppose, the size of source file (f1.txt) is 1 GB and the block 
> size is 128 MB. I want to copy the file in destination as follows:
> f1.txt.part1
> f2.txt.part2
> 
> By default, size of each part file is 128 MB except the last part.
> Design: Currently, the BlockWriter is restricted to write the part files into 
> the HDFS on which the app is running. To achieve the above use case, operator 
> needs the block index and relative path information. BlockMetadata which is 
> the input port for the BlockWriter doesn't have these information.
> So, I am creating the new operator(PartFileWriter) which extends from 
> BlockWriter with the input port of type FileMetadata.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Commented] (APEXMALHAR-2484) BlockWriter for writing the part files into the specified directory

2017-04-27 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/APEXMALHAR-2484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15986306#comment-15986306
 ] 

ASF GitHub Bot commented on APEXMALHAR-2484:


GitHub user chaithu14 opened a pull request:

https://github.com/apache/apex-malhar/pull/614

APEXMALHAR-2484 Support of PartFileWriter for writing the part files



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/chaithu14/incubator-apex-malhar 
APEXMALHAR-2484-partfilewriter

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/apex-malhar/pull/614.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #614


commit d45ea93d8db24af6181d55a5b4e2a13148e4629b
Author: chaitanya 
Date:   2017-04-27T10:00:09Z

APEXMALHAR-2484 Support of PartFileWriter for writing the part files




> BlockWriter for writing the part files into the specified directory
> ---
>
> Key: APEXMALHAR-2484
> URL: https://issues.apache.org/jira/browse/APEXMALHAR-2484
> Project: Apache Apex Malhar
>  Issue Type: Task
>Reporter: Chaitanya
>Assignee: Chaitanya
>
> Use case: Suppose, the size of source file (f1.txt) is 1 GB and the block 
> size is 128 MB. I want to copy the file in destination as follows:
> f1.txt.part1
> f2.txt.part2
> 
> By default, size of each part file is 128 MB except the last part.
> Design: Currently, the BlockWriter is restricted to write the part files into 
> the HDFS on which the app is running. To achieve the above use case, operator 
> needs the block index and relative path information. BlockMetadata which is 
> the input port for the BlockWriter doesn't have these information.
> So, I am creating the new operator(PartFileWriter) which extends from 
> BlockWriter with the input port of type FileMetadata.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[GitHub] apex-malhar pull request #614: APEXMALHAR-2484 Support of PartFileWriter for...

2017-04-27 Thread chaithu14
GitHub user chaithu14 opened a pull request:

https://github.com/apache/apex-malhar/pull/614

APEXMALHAR-2484 Support of PartFileWriter for writing the part files



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/chaithu14/incubator-apex-malhar 
APEXMALHAR-2484-partfilewriter

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/apex-malhar/pull/614.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #614


commit d45ea93d8db24af6181d55a5b4e2a13148e4629b
Author: chaitanya 
Date:   2017-04-27T10:00:09Z

APEXMALHAR-2484 Support of PartFileWriter for writing the part files




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] apex-malhar pull request #611: APEXMALHAR-2484 Support of PartFileWriter for...

2017-04-27 Thread chaithu14
Github user chaithu14 closed the pull request at:

https://github.com/apache/apex-malhar/pull/611


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


Re: Towards Apache Apex 3.6.0 release

2017-04-27 Thread Tushar Gosavi
All the issues marked as 3.6 have been resolved. I will go ahead and
prepare RC1 candidate.

Thanks,
- Tushar.

On Wed, Apr 26, 2017 at 2:30 PM, Tushar Gosavi 
wrote:

> These errors went away after switching to 1.7 version of java for build.
>
> Thanks,
> -Tushar.
>
>
> On Wed, Apr 26, 2017 at 12:50 PM, Tushar Gosavi 
> wrote:
>
>> I am following process specified at http://apex.apache.org/release.html to
>> prepare a release candidate for 3.6.
>>
>> I am getting error while executing command
>> mvn clean apache-rat:check deploy -Papache-release -DskipTests
>>
>>
>> 100 warnings
>> [INFO] 
>> 
>> [INFO] Reactor Summary:
>> [INFO]
>> [INFO] Apache Apex  SUCCESS
>> [06:51 min]
>> [INFO] Apache Apex API  FAILURE [
>>  5.255 s]
>> [INFO] Apache Apex Common Library . SKIPPED
>> [INFO] Apache Apex Buffer Server .. SKIPPED
>> [INFO] Apache Apex Stream Processing Engine ... SKIPPED
>> [INFO] Apache Apex Application Maven Archetype  SKIPPED
>> [INFO] Apache Apex App Configuration Maven Archetype .. SKIPPED
>> [INFO] 
>> 
>> [INFO] BUILD FAILURE
>> [INFO] 
>> 
>> [INFO] Total time: 06:56 min
>> [INFO] Finished at: 2017-04-26T12:41:35+05:30
>> [INFO] Final Memory: 74M/903M
>> [INFO] 
>> 
>> [ERROR] Failed to execute goal 
>> org.apache.maven.plugins:maven-javadoc-plugin:2.9:jar
>> (attach-javadocs) on project apex-api: MavenReportException: Error while
>> creating archive:
>> [ERROR] Exit code: 1 - /home/tushar/work/apex/core/ap
>> i/src/main/java/com/datatorrent/api/Attribute.java:116: warning: no
>> description for @param
>> [ERROR] * @param 
>> [ERROR] ^
>> [ERROR] 
>> /home/tushar/work/apex/core/api/src/main/java/com/datatorrent/api/Attribute.java:117:
>> warning: no description for @param
>> [ERROR] * @param key
>> [ERROR] ^
>> [ERROR] 
>> /home/tushar/work/apex/core/api/src/main/java/com/datatorrent/api/Attribute.java:146:
>> warning: no description for @throws
>> [ERROR] * @throws java.lang.CloneNotSupportedException
>> [ERROR] ^
>> [ERROR] 
>> /home/tushar/work/apex/core/api/src/main/java/com/datatorrent/api/AutoMetric.java:59:
>> error: self-closing element not allowed
>> [ERROR] * It aggregates metrics from multiple physical partitions of an
>> operator to a logical one.
>> [ERROR] ^
>>
>> What is the correct command to build the release.
>>
>> Thanks,
>> - Tushar.
>>
>>
>


[jira] [Commented] (APEXCORE-700) Make the plugin registration interface uniform

2017-04-27 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/APEXCORE-700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15986185#comment-15986185
 ] 

ASF GitHub Bot commented on APEXCORE-700:
-

Github user asfgit closed the pull request at:

https://github.com/apache/apex-core/pull/519


> Make the plugin registration interface uniform
> --
>
> Key: APEXCORE-700
> URL: https://issues.apache.org/jira/browse/APEXCORE-700
> Project: Apache Apex Core
>  Issue Type: Sub-task
>Reporter: Pramod Immaneni
>Assignee: Pramod Immaneni
>Priority: Minor
> Fix For: 3.6.0
>
>
> The user-facing plugin registration for DAG setup plugins is slightly 
> different from runtime plugins. It would be better to have a uniform way to 
> do this.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[GitHub] apex-core pull request #519: APEXCORE-700 Add Evolving annotations for class...

2017-04-27 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/apex-core/pull/519


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] apex-core pull request #519: APEXCORE-700 Add Evolving annotations for class...

2017-04-27 Thread tushargosavi
GitHub user tushargosavi opened a pull request:

https://github.com/apache/apex-core/pull/519

APEXCORE-700 Add Evolving annotations for classes changed through 985…

…6080ed

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tushargosavi/apex-core APEXCORE-700

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/apex-core/pull/519.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #519


commit ea0adae7209d9cd9d5586c38c2f5fc7b3e7cf8ab
Author: Tushar R. Gosavi 
Date:   2017-04-27T07:26:39Z

APEXCORE-700 Add Evolving annotations for classes changed through 9856080ed




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Commented] (APEXCORE-700) Make the plugin registration interface uniform

2017-04-27 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/APEXCORE-700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15986146#comment-15986146
 ] 

ASF GitHub Bot commented on APEXCORE-700:
-

GitHub user tushargosavi opened a pull request:

https://github.com/apache/apex-core/pull/519

APEXCORE-700 Add Evolving annotations for classes changed through 985…

…6080ed

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tushargosavi/apex-core APEXCORE-700

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/apex-core/pull/519.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #519


commit ea0adae7209d9cd9d5586c38c2f5fc7b3e7cf8ab
Author: Tushar R. Gosavi 
Date:   2017-04-27T07:26:39Z

APEXCORE-700 Add Evolving annotations for classes changed through 9856080ed




> Make the plugin registration interface uniform
> --
>
> Key: APEXCORE-700
> URL: https://issues.apache.org/jira/browse/APEXCORE-700
> Project: Apache Apex Core
>  Issue Type: Sub-task
>Reporter: Pramod Immaneni
>Assignee: Pramod Immaneni
>Priority: Minor
> Fix For: 3.6.0
>
>
> The user-facing plugin registration for DAG setup plugins is slightly 
> different from runtime plugins. It would be better to have a uniform way to 
> do this.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Resolved] (APEXCORE-594) Plugin support in Apex

2017-04-27 Thread Tushar Gosavi (JIRA)

 [ 
https://issues.apache.org/jira/browse/APEXCORE-594?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Tushar Gosavi resolved APEXCORE-594.

   Resolution: Fixed
Fix Version/s: 3.6.0

> Plugin support in Apex
> --
>
> Key: APEXCORE-594
> URL: https://issues.apache.org/jira/browse/APEXCORE-594
> Project: Apache Apex Core
>  Issue Type: New Feature
>Reporter: Tushar Gosavi
> Fix For: 3.6.0
>
>
> Investigate adding new functionality to apex through user plugins.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Resolved] (APEXCORE-700) Make the plugin registration interface uniform

2017-04-27 Thread Tushar Gosavi (JIRA)

 [ 
https://issues.apache.org/jira/browse/APEXCORE-700?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Tushar Gosavi resolved APEXCORE-700.

Resolution: Fixed

> Make the plugin registration interface uniform
> --
>
> Key: APEXCORE-700
> URL: https://issues.apache.org/jira/browse/APEXCORE-700
> Project: Apache Apex Core
>  Issue Type: Sub-task
>Reporter: Pramod Immaneni
>Assignee: Pramod Immaneni
>Priority: Minor
> Fix For: 3.6.0
>
>
> The user-facing plugin registration for DAG setup plugins is slightly 
> different from runtime plugins. It would be better to have a uniform way to 
> do this.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[GitHub] apex-core pull request #518: APEXCORE-700 Uniform interface between setup an...

2017-04-27 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/apex-core/pull/518


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Commented] (APEXCORE-700) Make the plugin registration interface uniform

2017-04-27 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/APEXCORE-700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15986133#comment-15986133
 ] 

ASF GitHub Bot commented on APEXCORE-700:
-

Github user asfgit closed the pull request at:

https://github.com/apache/apex-core/pull/518


> Make the plugin registration interface uniform
> --
>
> Key: APEXCORE-700
> URL: https://issues.apache.org/jira/browse/APEXCORE-700
> Project: Apache Apex Core
>  Issue Type: Sub-task
>Reporter: Pramod Immaneni
>Assignee: Pramod Immaneni
>Priority: Minor
> Fix For: 3.6.0
>
>
> The user-facing plugin registration for DAG setup plugins is slightly 
> different from runtime plugins. It would be better to have a uniform way to 
> do this.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


Join support in Malhar

2017-04-27 Thread Bhupesh Chawda
Hi Community,

Currently the support for join in Malhar is little fuzzy for the end user.
We have multiple implementations -

   1. Join Impl 1 - Inner Join implementation, based on Managed state
   2. Join Impl 2 - Merge operator, Windowed implementation, based on
   Spillable structures (based on managed state)

Following are the differences between the two:

   - As the name implies, Join Impl 1 is meant for inner joins, while Join
   Impl 2 has generic support for inner as well as outer joins.
   - Join Impl 1 supports sliding time windows with support for expiring
   old tuples. Join Impl 2 needs understanding of windowing concepts and uses
   watermarking support for functioning.
   - By looking at the implementations of managed state used by Join Impl 1
   and Join Impl 2, it seems like Join Impl 1 would have a performance
   advantage over Join Impl 2.

The purpose of this email is to see what can be done to simplify the join
usability in Malhar. Following are some options:

   1. Keep both implementations with clear documentation of the usability
   for both.
   2. Remove Join Impl 1 from Malhar and work with Join Impl 2 to improve
   performance. Note that even though Join Impl 1 addresses a very specific
   use case, it is the most common requirement in streaming join use cases.
   3. Any other option?

Thanks.

~ Bhupesh

​​