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 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 <t...@apache.org> wrote:
commit 9856080ede62a4529d730bcb6724c757f5010990
Author: Pramod Immaneni & Vlad Rozov <pramod+v.ro...@datatorrent.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 for the
users
and
hence asked for the changes.
On Thu, Apr 27, 2017 at 9:05 AM, Thomas Weise <t...@apache.org>
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
