apex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vlad Rozov <v.ro...@datatorrent.com>
Subject Re: PR merge policy
Date Sat, 29 Apr 2017 15:33:04 GMT
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 <v.rozov@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 <amol@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:amol@datatorrent.com | M: 510-449-2606 | Twitter: @*amolhkekre*
>>>>
>>>> www.datatorrent.com
>>>>
>>>>
>>>> On Fri, Apr 28, 2017 at 7:10 PM, Vlad Rozov <v.rozov@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: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 <pramod@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.rozov@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 <thw@apache.org>
wrote:
>>>>>>>
>>>>>>> +1 as measure of last resort.
>>>>>>>
>>>>>>> On Thu, Apr 27, 2017 at 9:25 PM, Vlad Rozov <v.rozov@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.rozov@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 <
>>>>>>>
>>>>>> 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 <thw@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 <
>>>>>>> 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 <v.rozov@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.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 <thw@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 <
>>>>>>>
>>>>>>> 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 <thw@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 <
>>>>>>>
>>>>>>> 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 <thw@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.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.
>>>>>>>
>>>>>>>
>>>


Mime
View raw message