Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 2302A200C65 for ; Sat, 29 Apr 2017 17:34:42 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 1FBA7160BA9; Sat, 29 Apr 2017 15:34:42 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id BE887160BA0 for ; Sat, 29 Apr 2017 17:34:40 +0200 (CEST) Received: (qmail 49249 invoked by uid 500); 29 Apr 2017 15:34:40 -0000 Mailing-List: contact dev-help@apex.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@apex.apache.org Delivered-To: mailing list dev@apex.apache.org Received: (qmail 49237 invoked by uid 99); 29 Apr 2017 15:34:39 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 29 Apr 2017 15:34:39 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id 2DF8DC0255 for ; Sat, 29 Apr 2017 15:34:39 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -1.496 X-Spam-Level: X-Spam-Status: No, score=-1.496 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, KAM_ASCII_DIVIDERS=0.8, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-2.796, RCVD_IN_SORBS_SPAM=0.5, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=disabled Authentication-Results: spamd4-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=datatorrent-com.20150623.gappssmtp.com Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id NkZr0ZPnT4cz for ; Sat, 29 Apr 2017 15:34:34 +0000 (UTC) Received: from mail-pf0-f170.google.com (mail-pf0-f170.google.com [209.85.192.170]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTPS id 5968D5F30B for ; Sat, 29 Apr 2017 15:34:34 +0000 (UTC) Received: by mail-pf0-f170.google.com with SMTP id q20so6998964pfg.0 for ; Sat, 29 Apr 2017 08:34:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=datatorrent-com.20150623.gappssmtp.com; s=20150623; h=subject:to:references:from:organization:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=1sFmejPlFlbEF3TCA8qJr2KNV2mPJHB9Lz/SNx7lQSk=; b=i3AdIJF5JuI9IH4STin1v9F2MJ+yuM7Lr10XPQirHXNkcZvqafM7R/WGsjmeTdlJp1 Jy1Dg1cbWf+toF1JGEkPrVJAG6Fx37SNiPEJeW87QXWOTYUr8snpeKJ/TSdkoKfOYVVW yoIE2rBLCxy2Qfx+d7nyR/TaFAm6YRTeat3nwPud5YIYZLU025CKKYKrORtsowgSo4C+ R5pspUgiFmwuKqFmuPiDzyoJPp8ZWT4meXhGq4FQyeyOtLaPjkIwqrPCqtlXzxkgXACD hFsHoquiohTxIzIk8V66cK5aPCDwLR5aOkZ2mjZA2yQzdBce+obtW7/M3z9hR9whJkNN eogA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-transfer-encoding:content-language; bh=1sFmejPlFlbEF3TCA8qJr2KNV2mPJHB9Lz/SNx7lQSk=; b=nYe6ViKVz93cRW1nTo+QjfFOx+Ok4cLBV34RlMWf70CdCD4kFRYfNzwrut5uzxNmog jcTxLoy0u8MqY/HIZK2OoGQSNNhGvPf/ttDDd4HUMafAELj7DPIEe2n63sGWSaHgrbdR GMkg4dnGr+Z9aIuMbCbWwWPD7qunfF9W8V/ocfWWlY/DKwkvHmjDFt7ri054S/5U3hSA HpJ0MVGn8qfsfTQGDUME663hc8ZUDYnIUSJkW07ayfq4uD9eKGGDvF1GlfMAziFgXjDy bcljOCWmF3t4cGpA7WgPkonTWOkMoTRGf242d7p+s7zdndWQ49rESIwIIAQKL2gfVzt4 hQTA== X-Gm-Message-State: AN3rC/65WsLkxI6BT228VfzzXpdeqWEIEvUXiFOp6yigxzTaTD4BRWZS vpgI98LoJwW7XIhLSXM= X-Received: by 10.84.236.67 with SMTP id h3mr22455796pln.86.1493480067010; Sat, 29 Apr 2017 08:34:27 -0700 (PDT) Received: from vrozov.local (c-71-204-190-196.hsd1.ca.comcast.net. [71.204.190.196]) by smtp.googlemail.com with ESMTPSA id y28sm16686224pfk.16.2017.04.29.08.34.26 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 29 Apr 2017 08:34:26 -0700 (PDT) Subject: Re: PR merge policy To: dev@apex.apache.org References: <7eb5beef-1f37-860f-276e-ebc290cffddd@datatorrent.com> <32fd73a6-3f1e-33dd-c64e-113969c0f3b1@datatorrent.com> <-417195641274065270@unknownmsgid> <84c2fd11-f476-d65b-2d5f-457431e28de5@datatorrent.com> From: Vlad Rozov Organization: DataTorrent Message-ID: <1246d801-f24b-d0ae-aa5d-48c6dfdaaedc@datatorrent.com> Date: Sat, 29 Apr 2017 08:33:04 -0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0) Gecko/20100101 Thunderbird/52.0.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US archived-at: Sat, 29 Apr 2017 15:34:42 -0000 Yes, the rollback is the last resort as Thomas mentioned. I hope that we will not need it. Possibly I misjudge, but I don't see forced push as an extremely disturbing event for the community especially if it is done quickly. Thank you, Vlad On 4/28/17 23:33, Bhupesh Chawda wrote: > ​ > Vlad, > > Your point regarding not merging any PR without allowing the community to > review is well taken. > I agree that the committer should be responsible for rolling back the > change and > ​I think ​ > we should establish the way in which it should be done. > > There > ​can > be a delay for the committer to notice that a commit should not have been > merged and needs to be reverted. But it might be a while before the > committer realizes this and a force push might create problems for the > community. Perhaps a new PR could be the way to go. > > Note that I agree that > ​undoing a PR > may not even be needed given that the community has had a chance to review > it, but there might still be cases where such undo commits need to be done. > > ~ Bhupesh > > > _______________________________________________________ > > Bhupesh Chawda > > E: bhupesh@datatorrent.com | Twitter: @bhupeshsc > > www.datatorrent.com | apex.apache.org > > > > On Sat, Apr 29, 2017 at 11:07 AM, Vlad Rozov > wrote: > >> If a committer is fast to pull the trigger to merge and slow to rollback >> when the policy is violated, the case can be sent to PMC. It will be a >> clear indication that a committer does not respect the community, so I >> disagree that this is "just kicking the can down the road" as committer >> right may be eventually revoked (hope we don't need to go that far ever). >> >> Not all policies are immediately reflected at >> http://apex.apache.org/contributing.html. The vote clearly happened a >> week ago when I initially proposed that a PR can be merged only once the >> community has a chance to review it . http://apex.apache.org/contrib >> uting.html is for new contributors and new committers, existing >> committers should be fully aware of the PR merge policy and if in doubt, >> raise a question here (dev@apex). >> >> We are all humans and may overlook problems in PRs that we review. The >> concern is not that Travis build may fail and a commit needs to be >> reverted. It will not be a valid reason to undo the commit (note that it is >> still committer responsibility to thoroughly review changes/new code and >> ensure that license is in place, build is free from compilation and other >> errors, code is optimal and readable, and basically put his/her name next >> to the contributor name). The concern is when a committer does not give >> community a chance for review. It is against "community before code" >> behavior that we want to prohibit. I am strongly for requesting a >> contributor to undo a commit in such cases and disallowing additional "fix" >> commits. >> >> Thank you, >> >> Vlad >> >> On 4/28/17 20:17, Munagala Ramanath wrote: >> >>> That's just kicking the can down the road: What if the committer is not >>> inclined >>> to perform the rollback ? Other reviewers can provide feedback on the >>> closed PR >>> but the committer may choose to add a new commit on top. >>> >>> Firstly, the point about waiting a day or two is not even a formal policy >>> requirement >>> outlined at http://apex.apache.org/contributing.html in the "Merging a >>> Pull >>> Request" >>> section, so it only has an advisory status currently in my view. >>> >>> Secondly, I think a variety of so called "violations" from overlooking >>> Travis build >>> failures, to not detecting missing unit tests, to not enforcing JavaDoc >>> requirements >>> are not fatal in most cases. Reverting commits in such cases is contrary >>> to >>> the >>> Apache injunction to "Put community before code" ( >>> http://community.apache.org/newbiefaq.html) >>> >>> So I am firmly against reverting except in the most dire circumstances and >>> in favor >>> of adding new commits to fix issues -- we do that for bugs, no reason to >>> treat these >>> other cases differently. >>> >>> Ram >>> >>> On Fri, Apr 28, 2017 at 7:24 PM, Amol Kekre wrote: >>> >>> That makes sense. The committer should fix upon feedback. PR policy >>>> violation is bad, I am not defending the violation. think if the >>>> committer >>>> takes the feedback and promptly works on a fix (new pr) it should be ok. >>>> >>>> Thks, >>>> Amol >>>> >>>> >>>> >>>> E:amol@datatorrent.com | M: 510-449-2606 | Twitter: @*amolhkekre* >>>> >>>> www.datatorrent.com >>>> >>>> >>>> On Fri, Apr 28, 2017 at 7:10 PM, Vlad Rozov >>>> wrote: >>>> >>>> I think it is a good idea to make the committer responsible for fixing >>>> the >>>> >>>>> situation by rolling back the commit and re-opening the PR for further >>>>> review. IMO, committer right comes with the responsibility to respect >>>>> the >>>>> community and policies it established. >>>>> >>>>> I would disagree that rolling back should be used only in case of a >>>>> disaster unless PR merge policy violation is a disaster :-) (and it >>>>> actually is). >>>>> >>>>> Thank you, >>>>> >>>>> Vlad >>>>> >>>>> On 4/28/17 14:21, Amol Kekre wrote: >>>>> >>>>> Strongly agree with Ilya. Lets take these events as learning >>>>> opportunities >>>>> for folks to learn and improve. There can always be second commit to fix >>>>>> in >>>>>> case there is code issue. If it is a policy issue, we learn and >>>>>> improve. >>>>>> Rolling back, should be used rarely and it does need to be a disaster. >>>>>> >>>>> We >>>>> need to be cognizant of new contributors worrying about the cost to >>>>> submit >>>>> code. >>>>>> I too do not think Apex is hurting from bad code getting in. We are >>>>>> >>>>> doing >>>>> great with our current policies. >>>>>> Thks, >>>>>> Amol >>>>>> >>>>>> >>>>>> E:amol@datatorrent.com | M: 510-449-2606 | Twitter: @*amolhkekre* >>>>>> >>>>>> www.datatorrent.com >>>>>> >>>>>> >>>>>> On Fri, Apr 28, 2017 at 1:35 PM, Ganelin, Ilya < >>>>>> Ilya.Ganelin@capitalone.com> >>>>>> wrote: >>>>>> >>>>>> Guess we can all go home then. Our work here is done: >>>>>> >>>>>>> >>>>>>> >>>>>>> W.R.T the discussion below, I think rolling back an improperly >>>>>>> reviewed >>>>>>> PR >>>>>>> could be considered disrespectful to the committer who merged it in >>>>>>> the >>>>>>> first place. I think that such situations, unless they trigger a >>>>>>> disaster, >>>>>>> should be handled by communicating the error to the responsible party >>>>>>> >>>>>> and >>>>> then allowing them to resolve it. E.g. I improperly commit an >>>>>> unreviewed >>>>> PR, someone notices and sends me an email informing me of my error, >>>>>> and I >>>>> then have the responsibility of unrolling the change and getting the >>>>>>> appropriate review. I think we should start with the premise that >>>>>>> we’re >>>>>>> here in the spirit of collaboration and we should create opportunities >>>>>>> for >>>>>>> individuals to learn from their mistakes, recognize the importance of >>>>>>> particular standards (e.g. good review process leads to stable >>>>>>> >>>>>> projects), >>>>> and ultimately internalize these ethics. >>>>>>> >>>>>>> >>>>>>> Internally to our team, we’ve had great success with a policy >>>>>>> requiring >>>>>>> two PR approvals and not allowing the creator of a patch to be the one >>>>>>> >>>>>> to >>>>> merge their PR. While this might feel a little silly, it definitely >>>>>> helps >>>>> to build collaboration, familiarity with the code base, and >>>>>> intrinsically >>>>> avoids PRs being merged too quickly (without a sufficient period for >>>>>>> review). >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> - Ilya Ganelin >>>>>>> >>>>>>> [image: id:image001.png@01D1F7A4.F3D42980] >>>>>>> >>>>>>> >>>>>>> >>>>>>> *From: *Pramod Immaneni >>>>>>> *Reply-To: *"dev@apex.apache.org" >>>>>>> *Date: *Friday, April 28, 2017 at 10:09 AM >>>>>>> *To: *"dev@apex.apache.org" >>>>>>> *Subject: *Re: PR merge policy >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> On a lighter note, looks like the powers that be have been listening >>>>>>> on >>>>>>> this conversation and decided to force push an empty repo or maybe >>>>>>> github just decided that this is the best proposal ;) >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Thu, Apr 27, 2017 at 10:47 PM, Vlad Rozov >>>>>> wrote: >>>>>>> >>>>>>> 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 < >>>>>>> >>>>>> ram@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 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 < >>>>>>> ram@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.rozov@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 < >>>>>>> >>>>>>> pramod@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 < >>>>>>> >>>>>>> pramod@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.rozov@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.rozov@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: ram@datatorrent.com | M: (408) 331-5034 | Twitter: @UnknownRam >>>>>>> >>>>>>> www.datatorrent.com | apex.apache.org >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> >>>>>>> _______________________________________________________ >>>>>>> >>>>>>> Munagala V. Ramanath >>>>>>> >>>>>>> Software Engineer >>>>>>> >>>>>>> E: ram@datatorrent.com | M: (408) 331-5034 | Twitter: @UnknownRam >>>>>>> >>>>>>> www.datatorrent.com | apex.apache.org >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> ------------------------------ >>>>>>> >>>>>>> The information contained in this e-mail is confidential and/or >>>>>>> proprietary to Capital One and/or its affiliates and may only be used >>>>>>> solely in performance of work or services for Capital One. The >>>>>>> information >>>>>>> transmitted herewith is intended only for use by the individual or >>>>>>> >>>>>> entity >>>>> to which it is addressed. If the reader of this message is not the >>>>>>> intended >>>>>>> recipient, you are hereby notified that any review, retransmission, >>>>>>> dissemination, distribution, copying or other use of, or taking of any >>>>>>> action in reliance upon this information is strictly prohibited. If >>>>>>> you >>>>>>> have received this communication in error, please contact the sender >>>>>>> >>>>>> and >>>>> delete the material from your computer. >>>>>>> >>>>>>> >>>