Hacker News new | ask | show | jobs
by connicpu 480 days ago
At my job we treat all warnings as errors and you can't merge your pull requests unless all automatically triggered CI pipelines pass. It requires discipline, but once you get it into that state it's a lot easier to keep it there.
3 comments

The last point is the key.

It then creates immense value by avoiding a lot of risk and uncertainty for little effort.

Getting from "thousands of warnings" to zero isn't a good ROI in many cases, certainly not on a shortish term. But staying at zero is nearly free.

This is even more so with those "fifteen flickering tests" these 23 tests that have been failing and ignored or skipped for years.

It's also why I commonly set up a CI, testing systems, linters, continuous deployment before anything else. I'll most often have an entire CI and guidelines and build automation to deploy something that will only say "hello world". Because it's much easier to keep it green, clean and automated than to move there later on

That's because it moves from being a project to being a process. I've tried to express this at my current job.

They want to take time out to write a lot of unit tests, but they're not willing to change the process to allow/expect devs to add unit tests along with each feature they write.

I'll be surprised if all the tests are still passing two months after this project, since nobody runs them.

That’s why TDD (Test-Driven Development) has become a trend. I personally don’t like TDD’s philosophy of writing tests first, then the code (probably because I prefer to think of a solutions more linearly), but I do absolutely embrace the idea and practice of writing tests along side of the code, and having minimum coverage thresholds. If you build that into your pipeline from the very beginning, you can blame the “process” when there aren’t enough tests.
The flip that switched for me to make me practice something TDD-adjacent is to replace most manual verification with writing a test. Once I got in the habit I find it so much faster, more consistent, and then I have lasting tests to check in!

I don't typically write tests first so it's not true TDD but it's been a big personal process improvement and quality boost.

> to allow/expect devs to add unit tests

For me such gigs are a red flag and immediate turn down (I'm freelancer with enough opportunities, luxury position, I know).

I would consider it really weird if management dictates exactly what tools and steps a carpenter must take to repair a chair. Or when the owner of a hotel tells the chef what steps are allowed when preparing fish. We trust the carpenter or chef to know this best. To know best how to employ their skills given the context.

If management doesn't trust the experts they hire to make the right choice in how they work, what tools they use, what steps they take, etc. that's a red flag: either they are hiring the wrong people (and the micromanaging is an attempt to fix that) or they don't think the experts are expert enough to make decisions on their own.

For me, this goes for tools (e.g. management dictates I must work on their windows machine with their IDE and other software) for processes (management forbids tests, or requires certain rituals around merges etc) and for internals (management forbidding or requiring certain abstractions, design patterns etc)

To be clear: a team, through, or via a management, should have common values and structures and such. And it makes perfect sense for management to define the context (e.g. this is a proof of concept, no need for the rigid quality here. Or we must get these features out of the door before thursday, nothing else matters.) It's when the management dictates how teams or experts must achieve this that it becomes a red flag to me.

I haven't been wrong in this. These red-flags almost always turned out to hint at underlying, deeply rooted cultural problems that caused all the technical troubles.

> I'll be surprised if all the tests are still passing two months after this project, since nobody runs them.

Wouldn't they just run as part of the build? At least for Java, Junit tests run as part of the build by default.

At my job we treat all warnings as errors and you can't merge your pull requests unless all automatically triggered CI pipelines pass. It requires discipline, but once you get it into that state it's a lot easier to keep it there.

Sounds like what we used to call "professionalism." That was before "move fast, break things and blame the user" became the norm.

It very much depends on the nature of your work.

If manual input can generate undefined behavior, you depend on a human making a correct decision, or you're dealing with real-world behavior using incomplete sensors to generate a model...sometimes, the only reasonable target is "fail gracefully". You cannot expect to generate right outputs with wrong inputs. It's not wrong to blame the user when economics, not just laziness, say that you need to trust the user to not do something unimagineable.

I think this is the kind of situation where a little professionalism would have prevented the issue: Handling uncaught exceptions in your threadpool/treemap combo would have prevented the problem from happening.

> That was before "move fast, break things and blame the user" became the norm.

When VCs only give you effectively 9 months of runway (3 months of coffees, 9 months of actual getting work done, 3 months more coffees to get the next round, 3 more months because all the VCs backed out because your demo wasn't good enough), move fast and break things is how things are done.

If giving startups 5 years of runway was the norm, then yeah, we could all be professional.

There's basically no proof that software used to be more "professional". Sure the process was more formal, but I've not seen any proof (and I'm not talking about peer reviewed stuff here, but even just anecdotal examples) of the "end result" of those processes being better, more robust or even less buggy than what we get out of what some may call "move fast and break stuff" development.
> professionalism." That was before "move fast, break things

I think you're professing a false dichotomy. Is it unprofessional to "move fast, break things"?

I'm a slow moving yak shaver partly due to concious intention. I admire some outcomes from engineers that break things like big rockets.

I definitely think we learn fast by breaking things: assuming we are scientific enough to design to learn without too much harm/cost.

> I admire some outcomes from engineers that break things like big rockets.

I work in unmanned aerospace. It started with 55lb quadcopters and got… significantly bigger from there.

I’ve thought a ton about what you’re saying over the last 5-6 years. I have broken things. My coworkers and underlings have broken things. We’ve also spent a bunch of time doing design review, code review, FEA review, big upfront design, and all those expensive slow things.

Here’s, for me, the dividing line: did we learn something new and novel? Or did we destroy kilobux worth of hardware to learn something we could have learned from a textbook, or doing a bit of math, or getting a coworker to spend a few hours reviewing our work before flying it?

And I 100% agree with your last statement: you can “move fast and break things for science” professionally. But… if something breaks when you weren’t expecting it to, the outcome of the post-mortem really should be surprising new knowledge and not “this made it to prod without review and was a stupid mistake”

> Is it unprofessional to "move fast, break things"?

Most of the time it is. The sorry state of software in general is a testament to that.

> At my job we treat all warnings as errors

Pretty sure what you actually mean is that you treat some warnings as errors, and disable the others. I would find it hard to believe you're building with literally all the compiler warnings enabled all the time.

We do have a couple disabled by default in the config, but it's still a lot of warnings:

    -Wall,-Wextra,-Werror,-Wno-type-limits,-Wno-attributes,-Wno-deprecated-declarations
And of course some are suppressed with a pragma for a short section of specific segments of the code where you can make the argument that it's appropriate during code review, but those pragmas stick out like a sore thumb.
> We do have a couple disabled by default in the config, but it's still a lot of warnings

A lot, yes, but definitely a lot more than 2 that you still don't have enabled. Might be worth looking into them if you haven't already -- you will definitely disable some of them in the process.

(And I assume you already have clang-tidy, but if not, look into that too.)

Yep, plus static analysis in CI. We also run ubasan, tsan, and valgrind on our unit tests.
-Wall + -Wextra doesn't actually enable all warning, though. There are quite a few others that you still have to enable manually.