Given the divergence of views on this topic, I think we need to clarify what "last resort" means; to me it means:
In most cases when PRs are prematurely merged, the preferred mechanism to address it will be to open a new PR with corrections resulting in a new commit on top. A forced reversion of the prior commit will only occur only when it is judged to be disastrously flawed. Ram On Sat, Apr 29, 2017 at 8:33 AM, Vlad Rozov <v.ro...@datatorrent.com> wrote: > 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: bhup...@datatorrent.com | Twitter: @bhupeshsc >> >> www.datatorrent.com | apex.apache.org >> >> >> >> On Sat, Apr 29, 2017 at 11:07 AM, Vlad Rozov <v.ro...@datatorrent.com> >> 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 <a...@datatorrent.com> >>>> 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:a...@datatorrent.com | M: 510-449-2606 | Twitter: @*amolhkekre* >>>>> >>>>> www.datatorrent.com >>>>> >>>>> >>>>> On Fri, Apr 28, 2017 at 7:10 PM, Vlad Rozov <v.ro...@datatorrent.com> >>>>> 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:a...@datatorrent.com | M: 510-449-2606 | Twitter: @*amolhkekre* >>>>>>> >>>>>>> www.datatorrent.com >>>>>>> >>>>>>> >>>>>>> On Fri, Apr 28, 2017 at 1:35 PM, Ganelin, Ilya < >>>>>>> ilya.gane...@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 <pra...@datatorrent.com> >>>>>>>> *Reply-To: *"dev@apex.apache.org" <dev@apex.apache.org> >>>>>>>> *Date: *Friday, April 28, 2017 at 10:09 AM >>>>>>>> *To: *"dev@apex.apache.org" <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 < >>>>>>>> v.ro...@datatorrent.com >>>>>>>> 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 <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 >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> ------------------------------ >>>>>>>> >>>>>>>> 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. >>>>>> >>>>>>> >>>>>>>> >>>>>>>> >>>> > -- _______________________________________________________ Munagala V. Ramanath Software Engineer E: r...@datatorrent.com | M: (408) 331-5034 | Twitter: @UnknownRam www.datatorrent.com | apex.apache.org
