Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

Doing analysis like this also has a huge impact on broken window theory. If engineers see a whole bunch of compiler warnings, then they don't think twice when they see just one more and it could be a really valid warning. It also gives a good sense of ownership and commitment to the codebase if everyone agrees to not check in code with warnings. Also, when you have new engineers copying and pasting code to get stuff working quickly, you certainly don't want them doing that with buggy code.


If engineers see a whole bunch of compiler warnings, then they don't think twice when they see just one more and it could be a really valid warning

As a real life example of how this can happen, PHP 5.3.something release had a serious bug with MD5 hashes essentially not working. (cf. https://bugs.php.net/bug.php?id=55439 http://www.php.net/archive/2011.php#id2011-08-22-1 ). Apparently there was a unit test for it, but there was ~200 failing unit tests, so they ignored it.


In my experience an even more pernicious problem is unit tests that are unreliable or too expensive (in time especially). Devs too easily come to allow unit test suites that routinely take reruns in order to get to 100% passing. Similarly, when build verification tests or CI tests get too bloated it can be hard to pick where to draw the line.


On my current project we have "Treat Warnings As Errors" set to true, so any warnings will cause the build to break. You can explicitly allow a specific warning with a pragma statement, if you can justify it.

It's made for a much cleaner codebase - and it means that we instantly know if we're about to do something iffy, rather than having them buried under a ton of other warnings.


At my last gig they had that as well, but still ignored it because the culture when I got there was totally full of broken windows.

There was 8000+ warnings and findbugs errors in the codebase that I went through and fixed. Luckily a lot were just white noise, but some >100 were definitely valid bugs that had been in the code for quite some time.

In order to keep things clean after all that work, I turned on warnings as failures as part of the continuous integration build (which I also set up) so that everyone would get an email each time the build failed. Yea public shaming, heh. The hard part was then training people to pay attention to the emails and not filter them to the trash.

As a final step, what I did was built the testing environment into the CI system so that if they wanted to test their code on a virtual machine before getting their branch into the latest iteration, they needed a clean build in order to generate the debian installers. That was the final kicker which really made people start paying attention to this stuff.

So, in order to push code to production, which everyone wants to do, you needed a clean build. Problem solved and it really cleaned up the quality of production releases. ;-)


Oh yes - we do a daily build that gets pushed out to testers - and if it's failing then they don't get it.

(And it took me an absolute age to get rid of all the warnings too.)


Just kind of curious, what happens if a developer fixes something and the tester wants it immediately so that they can test that fix? Do they have to wait another day for the build to happen?

I setup builds which could be tested as soon as the build was complete. I also had to do a lot of work to optimize the build process so it would complete quickly. It started out with ~20+ minutes and I worked it down to around 3-5 minutes.


We _can_ roll out builds faster - but as we're using multiple tiers of framework, we have to make sure that the back end is still compatible with the front end. The back end is only promoted once per day, and if there's been a breaking change since the last promotion then we can't move the front-end up until the back end is also promoted.

And as multiple front-ends use the same back-end, we can't just move the back-end up whenever we feel like it, or we break everyone else's UIs.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: