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 <r...@datatorrent.com> 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 > >>> > >>> > > > > > -- > > _______________________________________________________ > > Munagala V. Ramanath > > Software Engineer > > E: r...@datatorrent.com | M: (408) 331-5034 | Twitter: @UnknownRam > > www.datatorrent.com | apex.apache.org >
