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 <[email protected]> >>> 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. :) > 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 [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
