On Wed, Dec 21, 2016 at 05:33:20PM +0100, 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. > >> > > > > 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; > > 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? > > 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. > > > This could have easily been resolved by the end of the week if people had > > spent the time making fixes instead of complaining. > > Interesting that you are not supposed to fix warnings in code that you > have never seen before but others are.
Little side note: Claiming that you have to know a codebase for changing a==b to EINA_DBL_CMP(a,b) is NOT realistic :) > Furthermore, I think > > the fact that the people who complained about the warnings were not the > > ones fixing any of them highlights my point. > > Making things up does not help this discussion. Also as I am one of those who complained: I complained because i am working on patches for elementary, i am changing a few focus apis in there. So it is quite meaningfull if a warning comes up, but with searching in 100 Warnings for a single new is not quite handy. Also there is still the thing i have told you in the other mail, why now? And more important, why at all? Is there a issue? Greetings bu5hm4n > > 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 ------------------------------------------------------------------------------ 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
