mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anton Chernov <mecher...@gmail.com>
Subject Re: Should PR-860 (Use modernized range loops where possible) be reverted?
Date Tue, 20 Nov 2018 15:28:51 GMT
Hi Carin,

The discussion [1] was about whether to enable automatic checks on using
old behaviour in new PR's. Kellens PR [2] was about modernizing the actual
code itself and was not up for voting, thus could not receive any technical
veto votes.

Per the discussion (as I have understood it), we won't get veto votes if we
would enable the check on CI, if it would be treated as a warning.

Thank you for merging the PR in the first place. I see no reason for
reverting it.

Best
Anton

[1]
https://lists.apache.org/thread.html/b47f285a80bef47c5ead6c361614e338a0661f6c0c76196c1e3719c5@%3Cdev.mxnet.apache.org%3E
[2] https://github.com/apache/incubator-mxnet/pull/12356


вт, 20 нояб. 2018 г. в 15:24, Pedro Larroy <pedro.larroy.lists@gmail.com>:

> Hi all
>
> I think we have to make the clear separation between the thread votes
> on "uniformly adopting C++11 range loops in the MXNet project" and a
> PR which refactored code to be more legible and with improved variable
> names.
> Merging that PR doesn't imply that we have to uniformly adopt the
> previous proposal.  The PR was reviewed and approved by several
> people. I would keep the two topics separate, merging this PR doesn't
> prescribe any particular idiom for future commits or reviews.
>
> Pedro.
>
> On Tue, Nov 20, 2018 at 2:58 PM Carin Meier <carinmeier@gmail.com> wrote:
> >
> > My intent was to be helpful, but I think I may have merged this PR
> > yesterday too soon thinking it was approved and ready to merge
> > https://github.com/apache/incubator-mxnet/pull/12356
> >
> > I didn't see the connected dev discussion
> >
> https://lists.apache.org/thread.html/b47f285a80bef47c5ead6c361614e338a0661f6c0c76196c1e3719c5@%3Cdev.mxnet.apache.org%3E
> > where there were -1 votes, which I believe are vetos?
> >
> > So the question is confirm: should PR should be reverted?
> >
> > Sorry for any confusion,
> > Carin
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message