Re: [VOTE] When in Doubt, Wait 24 Hours Before Merging

Also, if a committer clicked "request changes" as their review (which shows near the CI build info), then they need to release that -- it can't be merged -- a "request changes" to me means "-1". On Thu, Feb 1, 2018 at 5:06 PM, Marco de Abreu wrote: > Additionally, I'd like to propose to add that

Re: [VOTE] When in Doubt, Wait 24 Hours Before Merging

Additionally, I'd like to propose to add that people with open requested changes actively have to be pinged and that the 24 hours only start after that ping. Was that part of your intention, Sheng? -Marco On Thu, Feb 1, 2018 at 4:29 PM, Nan Zhu wrote: > +1, but do not understand why we merged P

Re: [VOTE] When in Doubt, Wait 24 Hours Before Merging

I assume you mean after a call of "Is everything ok to merge?" There'd have to be some sort of trigger for ppl to know someone has started the 24-hour timer, right? On Thu, Feb 1, 2018 at 4:20 PM, Sheng Zha wrote: > Hi, > > In order to avoid having miscommunication and unaligned expectation,

Re: [VOTE] When in Doubt, Wait 24 Hours Before Merging

+1, but do not understand why we merged PRs which was not completely approved? On Thu, Feb 1, 2018 at 4:20 PM, Sheng Zha wrote: > Hi, > > In order to avoid having miscommunication and unaligned expectation, I'd > like to propose a lazy vote on a new rule for merging pull requests. > Specifically

[VOTE] When in Doubt, Wait 24 Hours Before Merging

Hi, In order to avoid having miscommunication and unaligned expectation, I'd like to propose a lazy vote on a new rule for merging pull requests. Specifically, for merging PRs, if there are open review comments and changes afterwards didn’t address the comments, we should have a grace-period of 24

[VOTE][RESULT] Creating release notes as part of PR merging process

Hello since there was no agreement on this topic and due to valid offline concerns about this creating a too big burden for the reviewers as well as bringing the risk that the notes would not be managed properly, I'll take this vote as failed and everything remains unchanged. Best regards, Marco

Re: Unit tests removed

I think I mean both explicit disagreement and potential ones that the contributor didn’t have a chance to express it yet. For example, your comments after #8302 have been merged, and Da even didn't have a chance to look into the revert PR, which is merged in an hour. I’m aware that the veto rule,

Re: Unit tests removed

What do you mean by "conflicted PR"? Do you mean disagreements in what can go in or out? If so, I believe that Apache addresses this. Per Apache: "A code-modification proposal may be stopped dead in its tracks by a -1 vote by a qualified voter. This constitutes a veto, and it cannot be overrule

Re: Unit tests removed

Hi Hen, I think both sides make sense to me 1) commenting out out-dated tests that are not being tested, 2) we should not lose any test. The real issue here is how to reach an agreement to avoid demotivating our contributors as much as possible, especially given they are "volunteers". It seems re

Re: Unit tests removed

Hi Kellen, The CPP tests PR totally make sense to me. What I really want to figure out is the process that reaching an agreement before merging conflicted PRs. Best Mu On Thu, Feb 1, 2018 at 11:25 AM, kellen sunderland < kellen.sunderl...@gmail.com> wrote: > Hey Mu + Da, sorry to hear that. I'

Re: Unit tests removed

Doing the right thing for code is the right thing. Feelings are in how we communicate, not in what happens to the code. If you are saying that the unit tests should be deleted, then good, but if the commit was in error then rolling it back seems fine to me. Given we’re volunteers we can’t rely on

Re: Unit tests removed

Hey Mu + Da, sorry to hear that. I've been working on the CPP tests PR and iterating on it for quite a while. I can assure you getting it merged had nothing to do with this revert from my perceptive. It's actually a task assigned to me for Q1 internally at Amazon. I'm actually completely unfami

Re: Unit tests removed

I think simply reverting the MKL PR that Da has been working for a half year and immediately enabling the cpp tests in the CI without reaching an agreement with Da seriously hurts his feelings. I put the details at the end of https://github.com/apache/incubator-mxnet/pull/9661 On Thu, Feb 1, 201

Re: Unit tests removed

I have created starter changes for converting to the CoreOpExecutor. This code compiles the first test. See notes in the PR description: https://github.com/cjolivier01/mxnet/pull/2 On Wed, Jan 31, 2018 at 9:47 PM, Zhao, Patric wrote: > Thanks, Chris, I agree the quality is the most important t

Re: Please Help Fix MXNet Licensing Issues for the next Release!

I think we should revert the license file to the previous, and improve from there. The policy is: "The LICENSE file MUST contain the full text of the Apache License 2.0 . When a package bundles code under several licenses, the LICENSE file MUST cont

Outstanding PR

Hi Can some of the admins please merge the following PR? I can't stand this warning on nvcc flooding my compilation anymore: https://github.com/dmlc/mshadow/pull/322 Thank you so much.