I am sorry but I am -1 on the force push in this case.
> On Apr 27, 2017, at 9:27 PM, Thomas Weise <t...@apache.org> wrote: > > +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 >>>>>> >>>>>> >>>>>> >>> >>
