Re: Proposal for treating warnings as errors in Linux & Clang builds (-Werror)

I agree, you don’t want to run tests until you feel the code is ready, but stopping everything to fix warnings that aren’t likely to cause any actual problems just slows down the process. I think that sanity checking makes perfect sense, but the usual way to do that is to first run some “smoke”

Re: Proposal for treating warnings as errors in Linux & Clang builds (-Werror)

@Christopher: I see your point, but the counter argument would be: "Why should the project run fairly expensive tests (~20 minutes on few GPU instances) for code that will require you to amend your commit anyway?" In normal circumstances I'd completely agree with you and let the full tests run

Re: Proposal for treating warnings as errors in Linux & Clang builds (-Werror)

While you can debate the "broken windows" theory (https://en.wikipedia.org/wiki/Broken_windows_theory) I think it has relevance to code warnings: the more warnings you tolerate, the more likely you are to end up with other undesirable things in your code. My preference has always been to treat

Re: Proposal for treating warnings as errors in Linux & Clang builds (-Werror)

I understand. What prevents you from disabling the -Werror flag on your build configuration? I don't see where's the big issue. We have already tens of flags to configure the build anyway. I have been fixing every warning that I came across so far in the main platforms that I have access. I

Re: Proposal for treating warnings as errors in Linux & Clang builds (-Werror)

So you're proposing to have a stage AFTER test execution which would report warnings as errors? While this is a good idea, I'm afraid that a fail-fast would also have its benefits - especially considering that compilation only takes a few minutes and consumes few resources while test execution

Re: Proposal for treating warnings as errors in Linux & Clang builds (-Werror)

Personally, I don’t like treating warnings as errors because it prevents compilation from completing and causes you to lose any ability to test the code and get any other information. Killing the build because of a failed warning for something that might not matter means that you may not find

Re: Proposal for treating warnings as errors in Linux & Clang builds (-Werror)

I'd vote for having warnings as errors only for CI but not in general builds which are getting executed by users on their local machine. Just in case CI misses a warning due to a different version, this could block a developer from compiling MXNet locally even though it might just be a warning

Re: Proposal for treating warnings as errors in Linux & Clang builds (-Werror)

Hi Chris I get the rationale of the point you raise, but In my opinion, and considering the complexity of C++ and the potential for difficult and expensive to track bugs, I think this should be enabled by default for both release and debug. A developer is free to disable warnings in his own

Re: Proposal for treating warnings as errors in Linux & Clang builds (-Werror)

It seems we have the `DEV` mode (https://github.com/apache/incubator-mxnet/blob/master/make/config.mk#L28) to do that? Is it correct? Xingjian From: Ziyue Huang Sent: Tuesday, January 16, 2018 9:01 AM To: dev@mxnet.incubator.apache.org

Re: Proposal for treating warnings as errors in Linux & Clang builds (-Werror)

+1 On Mon, Jan 15, 2018 at 8:02 PM Ziyue Huang wrote: > +1 since warnings might be potential errors. In my perspective, an > excellent project is better to have 0 warnings. > > Chris Olivier 于2018年1月16日 周二上午2:27写道： > > > If enabled, it should only

Re: Proposal for treating warnings as errors in Linux & Clang builds (-Werror)

If enabled, it should only cause errors in Release builds, since having warnings in WIP code is not unusual. In addition, different developers use different gcc/clang versions. Some gcc versions, for instance, generate warnings where others do not. It would not be fair to render unbuildable a

Re: Proposal for treating warnings as errors in Linux & Clang builds (-Werror)

+1 (binding) On Mon, Jan 15, 2018 at 9:43 AM, Marco de Abreu < marco.g.ab...@googlemail.com> wrote: > +1 > > On Mon, Jan 15, 2018 at 6:27 PM, Pedro Larroy < > pedro.larroy.li...@gmail.com> > wrote: > > > Hi > > > > I would like to propose to compile in CI with warnings as errors for > >

Proposal for treating warnings as errors in Linux & Clang builds (-Werror)

Hi I would like to propose to compile in CI with warnings as errors for increased code quality. This has a dual purpose: 1. Enforce a clean compilation output. Warnings often indicate deficiencies in the code and hide new warnings which can be an indicator of problems. 2. Warnings can surface