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 <zhash...@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.li...@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. > >
