+1 as measure of last resort.

On Thu, Apr 27, 2017 at 9:25 PM, Vlad Rozov <v.ro...@datatorrent.com> 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 <v.ro...@datatorrent.com>
>> 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 <r...@datatorrent.com
>>>> >
>>>> 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 <t...@apache.org> 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 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.rozov@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
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> --
>>>>>>>
>>>>>>> _______________________________________________________
>>>>>>>
>>>>>>> Munagala V. Ramanath
>>>>>>>
>>>>>>> Software Engineer
>>>>>>>
>>>>>>> E: r...@datatorrent.com | M: (408) 331-5034 | Twitter: @UnknownRam
>>>>>>>
>>>>>>> www.datatorrent.com  |  apex.apache.org
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>
>>>>> _______________________________________________________
>>>>>
>>>>> Munagala V. Ramanath
>>>>>
>>>>> Software Engineer
>>>>>
>>>>> E: r...@datatorrent.com | M: (408) 331-5034 | Twitter: @UnknownRam
>>>>>
>>>>> www.datatorrent.com  |  apex.apache.org
>>>>>
>>>>>
>>>>>
>>
>

Reply via email to