On 12/21/2016 11:56 AM, Stefan Schmidt wrote: > Hello. > > On 21/12/16 17:46, Christopher Michael wrote: >> On 12/21/2016 11:33 AM, Stefan Schmidt wrote: >>> Hello. >>> >>> On 21/12/16 17:02, Mike Blumenkrantz wrote: >>>> On Wed, Dec 21, 2016 at 5:11 AM Stefan Schmidt <ste...@osg.samsung.com> >>>> wrote: >>>> >>>>> Hello >>>>> >>>>> On 20/12/16 17:36, Mike Blumenkrantz wrote: >>>>>> I think your "How many real issues have you seen" was the same argument >>>>>> against running any static analysis a couple years ago; now we have >>>>> weekly >>>>>> reports for that. >>>>>> >>>>>> I have seen bugs that resulted from illegal float comparison. The fact >>>>> that >>>>>> a warning may be a false positive in some or even most cases does not >>>>>> ensure that every warning is a false positive. >>>>>> >>>>>> Given that we are so pedantic about warnings for much more trivial >>>>> matters >>>>>> (e.g., -Wunused-parameter as part of -Wextra), it seems bizarre to me >>>>> that >>>>>> anyone would complain about enforcing valid comparisons for floats. >>>>> >>>>> Getting the code in shape for correct float comparisons is actually >>>>> something I would like to see. >>>>> >>>>> The real problem here is/was how this was handled. Forcing the compiler >>>>> flag to everyone's build is the problem. That and seeing Chris and >>>>> >>>> Cedric going crazy and doing nearly 100 patches in a short timeframe. >>>>> And even after all this my build was still noisy with all these warnings. >>>>> >>>> >> >> Your build was (likely) still noisy because we had not finished removing >> all of the warnings... >> >>>> If people want to spend a few hours fixing hundreds of compile warnings >>>> before leaving on holiday then I think they should be commended, not held >>>> up as examples of what not to do. There's nothing wrong with creating a lot >>>> of patches; >>> >> >> Agree. >> >>> I'm not complaining about the amount of patches, I don't want this fixed >>> my one big commit either. And if this would have been it and all >>> warnings fixed I would not reverted anything. Given that Chris even >>> reverted patches because he did not compile test them before pushing I >>> really wonder what this rush is about. Is this a battle on getting >>> patches in before vacation, or what? >>> >> >> The revert was mainly due to ethumb issues. Fixing the initial float >> warning actually caused other warnings do to the usage of CHECK_DELTA >> macro. Yes, I did not compile test the ethumb one (hence why it was >> reverted) .. that does NOT mean that others were not compile tested. >> >>> it's not like anyone here is taking the time to review all >>>> these commits once they hit the repo. >>> >>> Just not true. I look over patches coming in over the commit mailing >>> list. And I know Tom also did. Others I can't say, but neither can you. >>> >>>> >>>>> >>>>> The reason we are so pedantic with the other warnings from the default >>>>> flags is that we want to have a clean build output to _actually spot >>>>> warnings from new code_ we are working on. >>>>> >>>> >>>>> I reverted the patch putting it into the default compiler flags. Enable >>>>> it locally, get the the amount of warnings down to a sane amount and >>>>> I'm all for putting it in again. (tests and examples are also full of >>>>> it. Mentioning it here because I know how much people love to avoid >>>>> compiling those.) >>>>> >>>> >>>> I'm supposed to enable a flag locally and fix hundreds of warnings by >>>> myself in code that I've never seen before? Or perhaps you think that >>>> others will collaborate and do this voluntarily if they have to manually >>>> enable a warning flag? Please be realistic. >>> >>> I am. You could have written a mail highlighting some examples why this >>> warnings is useful, you could have started a branch with it enabled and >>> worked together with Chris and Cedric on it to fix the warnings. Nothing >>> of this happened. Out of the blue this warning was enabled and a rush of >>> patches came in. You should not be surprised that people bring this up. >>> >> >> The "rush" of patches coming in was mainly because I HATE COMPILER >> WARNINGS :) and wanted to get some of them fixed. Would you have > > Over 40 of them have been from Cedric. Its not all about you hating > compiler warnings. :) >
Fair enough :) >> preferred that the flag be enabled and nobody fix any of it ?? I say be >> thankful that someone cared enough to start fixing them... > > I already wrote it two times but it seems things only stick the third > time... > I would wanted it to be enabled locally by people if they want to work > on it, fix the warnings, compile and test, push the patches and once the > noise have reached a manageable level enabled it by default for everyone. > > regards > Stefan Schmidt > ------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today.http://sdm.link/intel _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
