mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pedro Larroy <pedro.larroy.li...@gmail.com>
Subject Re: warnings as errors
Date Wed, 22 May 2019 20:50:53 GMT
I was not able to fix the warnings on mshadow type switch with unused
local typedefs, that's one example of warning that I would disable. I
couldn't find a way to solve that one and I think the ramifications of
an unused typedef are not likely to cause bugs in the code and are
more of a pedantic nature.

https://github.com/apache/incubator-mxnet/pull/13424

I think turning on them one by one is going to pollute the compilation
output unecesarily and even run into commandline length problems?  I
think is best to enable all warnings and errors and cherry pick the
ones we can't fix or won't fix on purpose.

In this other case, I managed to tighten the warnings but ASAN is
giving some problems:

https://github.com/apache/incubator-mxnet/pull/14850

I think having warning fixes reviewed and merged faster without
triggering additional refactorings could make this process easier,
also having some help in this area and contributions would be greatly
appreciated.

Pedro.

On Tue, May 21, 2019 at 3:49 PM Sheng Zha <zhasheng@apache.org> wrote:
>
> It would be great to enforce the check for warnings and treat as errors. Some questions
I have:
> - what are the warnings that you think should be ignored?
> - for the rest of the warning types, can we turn them on one by one?
>
> -sz
>
> On 2019/05/21 22:33:51, Pedro Larroy <pedro.larroy.lists@gmail.com> wrote:
> > Hi dev@
> >
> > I try to fix any warning that I see during compilation of MXNet in my
> > platform and with the build toggles that I care about. These seemingly
> > trivial and ungrateful efforts, take nonetheless energy on the
> > contributor side.
> >
> > I think overall I submitted myself more than a dozen of PRs fixing
> > warnings and I would like to call for additional help and
> > contributions in this area.
> >
> > There was a question from Lin about discussing this on the mailing
> > list, I have the feeling that everybody agrees on moving towards zero
> > warnings and warnings as errors. I think there are unavoidable
> > warnings that can be disabled specifically such as the one triggered
> > by mshadow type switch.
> >
> > Some important missing warnings such as warning on missing return
> > values (ie. forgetting to return on a function returning non-void)
> > cause bugs, danger and additional time spent bugfixing, which can be
> > better spent somewhere else.
> >
> > Is there a process that we can figure out such as a more expedited
> > merges of PRs fixing warnings or a specific label?
> >
> > Some simple PRs that fixes a warning can take long to merge, and
> > sometimes trigger too much discussion and make the progress a bit
> > unfriendly to contributors.
> >
> > Any help or constructive ideas on this topic would be appreciated.
> >
> > Pedro.
> >

Mime
View raw message