| Thread Previous • Date Previous • Date Next • Thread Next |
Le 04/01/2013 09:05, Didier Roche a écrit :
Le 03/01/2013 04:38, Stephen M. Webb a écrit :On 01/02/2013 09:43 PM, Daniel van Vugt wrote:Did the compiler really give a warning? As far as I can tell, Nux, Unity and Compiz already use "-Wall -Werror". If mistakes still slip past gcc (and they do) then I recommend making your code compatible with clang because it gives more and better warnings/errors. We've done this for lp:compiz but I'm not sure if Nux and Unity are clang-friendly yet.The compiler is unlikely to have given a warning in production builds since Unity, at least, does not compile with -Wextra (or -Wunused). My guess is this level of warning was not enabled because of the sheer amount of noise it generates (which is itself a signal that should not be ignored). We have it on our list as a low-priority item to be able to build with -Wextra enabled but it's hard to justify thousands of lines of code churn when expanding and improving overall unit test coverage will give us a better return on investment of our limited resources. I believe that a proper test harness and test coverage would have caught the regression. Almost all of the test cases I have seen going in to Unity in the last few months have been to verify new functionality or regression tests for bug fixes, and that's an improvement over past practices, but without a validation test suite we're still using the Edit and Pray methodology. The start of a new calendar year seems like a good time to start a renewed push for Quality in Unity. I am going to make expanded test coverage a priority part of the desktop Unity polish for 13.04 and I think the investment will pay off going forward with other form factors.Hey guys, Yeah, we need to push back quality at the heart of our process.Just back from vacations, I have the unpleasant surprise to notice that unity fails to build in the staging ppa since 2012-12-19 and that more than 7 merges were done in between without fixing it first (or reverting the faulty merge): https://launchpad.net/~unity-team/+archive/staging/+packages?field.name_filter=unity&field.status_filter=superseded&field.series_filter=raring <https://launchpad.net/%7Eunity-team/+archive/staging/+packages?field.name_filter=unity&field.status_filter=superseded&field.series_filter=raring> I guess it's failing due to the precompiled header: http://bazaar.launchpad.net/~unity-team/unity/trunk/revision/3002 <http://bazaar.launchpad.net/%7Eunity-team/unity/trunk/revision/3002>. Unity is built in parallel in the ppa, that's maybe why it passed the merger step.Mirv is now looking and is taking action to get that under control again, but please please, when you do merge something, track the builds and install your branch locally to check for side-effects. For this one, if we couldn't get that fixed easily, the quickest path at the time would have been to revert ASAP the branch until we find a good fix and not having an unreleasable unity state as we have right now.
Ok, this is now fixed/workarounded after some testing. There were 3 causes:- precompiled header/cmake seems to act badly together (with the way cmake is passing -g -O2), workarounded for now disabling pch support. Opened bug ( https://bugs.launchpad.net/unity/+bug/1095978) and Satoris will have a deeper look. - Nux broke its API and ABI and the build-dependency requirement wasn't bumped on unity - Compiz broke its API and ABI and the build-dependency requirement wasn't bumped on unity
Please, for the 2 laters, please bump the requirement on the depending package once you break the API. This is extremely important (and not the first time I have to do it myself) whereas quite easy to do in debian/control as you can see in the attached merge request: https://code.launchpad.net/~didrocks/unity/fix-ftbfs-2013/+merge/141876 <https://code.launchpad.net/%7Edidrocks/unity/fix-ftbfs-2013/+merge/141876>
Thanks everyone for your cooperation, Cheers, Didier
| Thread Previous • Date Previous • Date Next • Thread Next |
