Perhaps some of the more productive workers are the ones that don't hesitate to make necessary code changes, test the changes adequately, and then move on to the next thing.
I have noticed that pretty much every software engineer to some degree has problems that they procrastinate on. Folk can spend 10x more time talking about doing something than it takes to do it. For hard problems, that discussion is necessary and beneficial, but lots of problems just need someone to open up the text editor and get it done.
I worked as a contractor with some companies and peer coded with their engineers.
What I found was that its not just procrastination. Many folks are just afraid to commit code, like literally scared and I could never get a real reason for that. At one point I added some code based on the direction what requirements were taking. But I could not convince him to commit it.
So we finally agreed to let it be there commented out, only a week later to find we need it now.
Sounds about right! I think that's an indication of lack of sufficient regression test coverage, coupled with complacency/fatigue from working on the same code base for an extended period of time.
Unfortunately, the most business critical pieces of code tend to be the least regression tested. It's the earliest stuff that was made before any test frameworks were matured, it's been hacked on countless times by half the team based on shifting requirements to the point that no one understands it fully, and any future changes are such a high priority that it's "faster" to test it manually or in production.
I am of course guilty of that myself on some pieces of code. I try to prioritize cleaning up expensive tech debt, though. When folk are hesitant to modify a piece of code, it's a strong indication that the code is due for refactoring. It's always worth it as long as you implement regression testing in the process.
Cause reasons for fear are not something people would tell you. I was in similar state twice and neither time I would be eager explain that to someone external.
I was "afraid" to commit in a team where code review evolved into huge micro-management with inconsistent requirements on what good code looks like. The reviewer iteratively forced you to change it again and again each time with "this is bad code cant go in" comments. But you could not learn what he considers good code, cause it was different every time. I left after.
Second time I was "afraid" to commit to part of code when our main architect had completely disproportionate blow up over previous bug. I made easy to fix bug which was my mistake. But it turned into massive public blow up over work being shitty, us intentionally ignoring his needs, there being tons of bug unusable version (there was literally one bug). Then he wanted massive refactoring to avoid possibility of same bug ... and I made bug in refactoring which led to same blow out, again claiming it was done without care etc.
After that, I really did not wanted to do any changes in that code. I cant guarantee complete lack of bugs in my code. Other people do bugs too for that matter, I dont think I make so much more of them. But, he was under pressure and stress that had nothing to do with me and I became rod for that.
That sounds miserable. I think, as a reviewer, it's important to keep in mind that most problems have many solutions. You have to be flexible and work with the person that did the hard work of implementing it. Everyone messes up and lets a bug slip through every now and then. That's what multiple layers of thorough testing is for.
On my current team, I'm considered to be the most thorough code reviewer. Folk usually thank me rather than scorn me, though. Some are afraid to send me their PRs not because they fear my feedback, but because they think reviewing their code will take up too much of my time. Reviewing code thoroughly doesn't take much time at all, though, if you make it a habit. If it's a bug, I explain my concern and suggest a fix. If it's a style nit, I link to the relevant style guide section. If it's a suggestion or personal preference, I explicitly say that, write out my rationale, then offer to chat more. If it's a weak suggestion, I tell them right off the bat that I'm fine either way. If it's a strong suggestion, I try to give them an "early out" by suggesting they simply add a // TODO comment. I'll let a coworker get away with murder as long as they leave a TODO comment.
For new team members that send me a PR for the first time, I typically send a message at the start describing what they should expect. That helps a lot, because everyone's first few PRs are going to be rough until they've gotten up to speed on the existing team's expectations.
I will say that there are some engineers that just don't take technical feedback well. When they join an existing team, they can be stubborn and refuse to adapt to the established culture. Instead, they either misinterpret criticism as personal attacks, or get frustrated and insist that the team conform to their preferences right off the bat. Team culture can and should evolve over time, but it requires respect and understanding of the status quo. It's possible for an open minded engineer to join a team, embrace its current culture, then radically change it all over the course of a few months. It's possible for a close minded engineer to not get more than a dozen PRs approved over the course of a year. Not to say that such engineers are good or bad one way or another, but rather folk should seek out projects that are compatible with their personality.
Also, it helps a lot to hammer out a lot of design and implementation details before writing any code. For any work project I start that I anticipate taking more than a few hours to code, I spend an hour or so writing an "architecture document" which I then distribute to anyone I think will have an opinion about it. That gives folk a place to ask questions and "bike shed" the problem long before I've invested any "artesianal coding energy". By the time the code reviewer looks at the code, they know what it's supposed to do and why it was written that way. As a pleasant bonus, I usually code about 3x faster when I have the document to reference.
I try to get new hires into this habit as early as possible to varying degrees of success. The folk that embrace it tend to be very productive; I don't know if it's causation or just correlation though. I have a slide deck to emphasize function over form, titled "Writing Mediocre Architecture Documents". It's a cult classic hit.
My question in regard to this is how do you write an architectural doc if you're exploring the problem space? If it takes 5h to explore, experiment and find the solution. And it takes 0.5h to write the doc. And 0.5h to make the change after you know the solution and have reverted the experimental/exploratory changes?
At that point is the doc necessary as a precursor to the change. By that I mean, do you send it out for feedback first and wait OR do you send it out/add that as a comment on you PR and push up the final change?
Additionally how do you know you're going to get feedback at all from sending out the doc? In my experience people are always busy with their own work and sending them a big chunk of text...well I wouldn't expect a response quickly compared to a short 1-2 line question
I consider any code that's written to be "speculative" until its design has been vetted by another engineer. By speculative, I mean that there's no expectation that the code will be approved and merged in its present form. An architecture document is simply a good way to get that buy-in. If I write down what I plan on doing in a document, and then solicit feedback on it from a coworker, there's a very good chance that the same coworker will approve the resulting code without anything but bug and style fixes. If you're exploring the problem space, yeah of course go ahead and hack away at some code. If the experiments go exceedingly well and you end up with production quality code, go ahead and throw it in a PR for review. You have to be receptive to "bike shedding" feedback, though, since the PR is the first opportunity you gave for folk to give that type of feedback.
It's also perfectly reasonable to write and distribute a document that effectively says, "I have no idea what the requirements are, but I have an arbitrary idea for how we should proceed anyway." More often than not, your coworkers will help flesh out the requirements, or agree with your arbitrary design decisions. Either way, it's a lot easier to have that type of discussion on a free-form wiki page rather than in a github PR that you already spent 4 hours getting to compile and pass test suites.
Re: soliciting feedback, even verbose architecture documents tend to be relatively easy for your coworkers to digest. It is communication written for human consumption rather than machine consumption, after all. For one thing, you can have a lot of fun with them. One of my recent arch documents had at least a dozen MC Hammer puns in it. I had no trouble getting anyone to read through that. Also, if you give your coworkers the opportunity and they don't follow up on it, it does give you the high road in any eventual PR contention. "I mentioned this a week ago and asked you for feedback on it, why are you making a fuss about it now?" Not that you should be setting up your coworkers like that. You should make a good faith effort to solicit feedback, and keep politely pestering folk until they get to it. Until you get buy-in, any code written is still speculative, even if you tried your best and your coworkers let you down.
It helps to lead by example. If you prioritize giving feedback to coworkers over your own implementation work (reading their documents, doing their code reviews in a timely way, etc.), then your coworkers will tend to notice and reciprocate.
Nah, but here's the content. You'll have to add your own clip art hastily pulled from google image search:
---
What’s the point?
Architecture documents are for you, not for everyone else
* Adds structure to chaos
* Makes feedback progressive rather than regressive
* You get to tell people to “Read The Friendly Manual”
---
But there’s so much boilerplate!
So don’t fill out the useless parts...
* Delete sections that aren’t relevant
* If there’s a better format, just do it that way
* Complain to <template owner> if the template is silly
---
The requirements are too ill-defined to write down
I’m sure writing code will solve the problem then. /sarcasm
* This is exactly why architecture documents are helpful
* Just barf out some bad requirements and ask for feedback
* If no one has any ideas, we’ve got bigger problems
---
I’m the one writing the code, back off!
Software engineers are experts about everything and tend to have opinions.
* Enumerate alternative approaches and politely explain why they are dumb
* Detail out the chosen approach
* When in doubt, write out function signatures
---
When is enough enough?
When it starts to feel passive aggressive
* Writing code without an architecture doc is speculative
* “Wasting” 30 minutes on a doc can save hours in a PR
* I’ve never regretted writing an architecture document
---
Super Fun Activity Time
Whatever you’re currently working on, write an architecture doc for it.
(If you already have one, quit wasting time and go write code!)
> where code review evolved into huge micro-management with inconsistent requirements on what good code looks like.
In my org this can be somewhat of a problem (different teams have different code style guidelines for long historical reasons which can make cross-team changes tricky).
One thing that helped a lot with this is publishing these guidelines in a central location, but more importantly we also made bots that reviewed every code review to point out common mistakes that people made, so reviewers could focus on the change itself instead of naming/spacing nits.
I know I've been testy with a coworker or junior before but its so counter productive to have a workplace with fear. Process can lead to trust and confidence so I try to focus on identifying and improving that (as well as mentoring around risky patterns and such.) Test, reviews, tooling etc. If simple mistakes are effecting the team, its really a process failure.
Obviously these are people problems. If my next team suffers from your first problem, I might try to ease it by introducing a code formatting tool like prettier. I don't always love the choices those tools make, but I do love eliminating that class of PR comment.
It doesn't eliminate the people problem, but it does limit its scope.
Commented out code is a terrible idea. It is the reason you look at source code and see it filled with garbage. When you have a developer who has no confidence in their code then you ask the usual question about requirements, testing, code review, etc. If it's still not committed then there's clearly a developer issue to workout with management.
When a developer fails to follow through on their tasks and takes extra time then this increases the budget of a project.
I understand I have put that up almost without any context, so let me add some.
It was(is) a (very) huge project which is still in development (my last interaction was more than 3 years ago) being built by a very big company in tech space, and various parts of it were outsourced to various vendors (including Indian software shops) and I was consultant to one such vendor. There were huge number of layers of management, while all the usual questions you mentioned would have the answer "yes".
So yeah the devs were in general scared about the code and management both. They wouldn't ask questions, just say "yes" to everything.
That's also a sign they aren't comfortable with the version control system. I used to be like that, and with cvs and svn it was somewhat justifiable. Now that we have git and it's everywhere I'm far less anxious. Not that git is perfect and things don't happen, but it's extremely good at not Screwing Up. It may not do exactly what you expected, but it doesn't break the repo. It also tries really hard to never forget anything unless you make an effort to force it. It might take a bit of digging around in the reflog to figure out how to get back to a sane state, but in every case I've had to deal with it was possible.
I'll give a coworker a hard time for making large infrequent commits, but I've never seen someone afraid to commit code. This sounds like the value proposition for version control hasn't really clicked for them.
I think it‘s less about version control and more about that the change is then associated with the employees name and if something ever goes wrong, it would be possible (easy?) to blame him therefore he‘s being scared about doing something because it could cause trouble for him somewhen in the future.
And, imho, that goes back to not enough testing nd no safety nets to check for code errors (like code review, static analysis, ...).
Clearly I am living in a bubble, because I cannot imagine working somewhere where I would rather be seen doing literally nothing than to do the exact thing I was hired for.
By commit, I think the intended meaning is "put on a path that will ultimately affect production".
For my current project, merging bad code typically means breaking a bunch of regression test suites for any coworkers that cut their branches at the wrong time. In more nefarious outcomes, it means a delayed software version release and, potentially, damaged UAV hardware at our test site.
For my previous project, merging bad code could have resulted in someone losing control of a fully drive by wire car in a closed parking lot or test track. That's what big red buttons are for!
For my project before that, some of my code was involved in handling literally every single sensor and actuator on a rocket and the space capsule on top of it. A subtle bug that slipped through to production could have been rather serious indeed.
If individuals are fearing the repercussions from doing their jobs on a safety critical system like that, then the process has already failed.
Your process needs to be so bulletproof that everyone involved feels absolutely sure that a defect will be caught. If there's any doubt in their minds then that part of the chain needs to be addressed and corrected.
I'm not saying that people shouldn't take it seriously or should get sloppy, but if you're worried that making a commit could end in disaster then you're moving too fast.
> Folk can spend 10x more time talking about doing something than it takes to do it
When I see this happening in the team, I immediately try to get the engineers to implement 90% to then talk about the other missing 10%. There are a million reasons why a solution isn't perfect and we need to put as much thought into it as possible. At the same time we have to keep in mind that everything we do is a tradeoff. If you think your solution is perfect you probably lack knowledge.
I prefer to ship 90% and then see how we can improve on that with data I can't produce if I don't ship anything. Talk about hypothetical future problems going in circles doesn't solve problems we have right now.
Don't get me wrong, I'm one of those 10x thought times guy myself. But I know how to keep the ball rolling in a team and take the responsibility for these types of decisions which look like educated corner cutting.
I think this points depends heavily on the complexity of the project. If the project is very complex (e.g. P2P, blockchain, distributed messaging, machine learning, ...) then you actually want engineers to think really hard about every line of code they write.
The best, most long-lasting code I ever wrote took a long time to write. Sometimes I have to think about a single small feature for multiple days before I begin to implement. Some foundational structural technical decisions require weeks of thinking and analyzing. It's the best way to guarantee that you don't have to come back to rewrite the code later.
When I was younger, I would refactor some of the foundation logic every few months as I added more changes on top. These days I almost never refactor the foundations. That extra time is totally worth it. It's very hard to come up with a good design.
i think this and discipline are key aspects. i see average performing engineers code by assumption. or knowing the “right way” to do something, which may take a lot of set up and refactoring and testing, and thinking no it’ll be too long. but then your code goes to review and it takes 2-4 days and 3 rounds to get approved. maybe that extra 2 hours of coding time would pay off over 4 days. i think the top people have experienced this and know it and do all of this in their first pass unprompted
I have noticed that pretty much every software engineer to some degree has problems that they procrastinate on. Folk can spend 10x more time talking about doing something than it takes to do it. For hard problems, that discussion is necessary and beneficial, but lots of problems just need someone to open up the text editor and get it done.