In the world of Software Development, there are often differing views on how to arrange teams. Regardless of the approach, people will leave/join over time, but team members need to be replaced and teams need to adapt.
There was a time when we were arranged into teams that were assigned to a Project, then moved onto a completely different one once complete. Any bugs introduced by the projects then get assigned to a “Service Improvement” team who only deal with bugs (and possibly ad-hoc user requests).
Then after a few years, and maybe under a new Development manager, they would restructure to Domain teams where you take ownership of a group of features and only projects related to those would be assigned to your team. Any bugs introduced by the projects stay with the team, which gives you greater incentive to fix them early as possible. People build up knowledge of their areas and become experts.
Then a few years later, we will switch back to Project teams.
There’s pros and cons to each structure, and there’s always edge cases which pose a management problem. Even in a Domain Team, there will be certain features that don’t neatly fit into the groups you defined, or ones that apply to many modules eg Printing.
Sometimes we have called a team that handles the miscellaneous features “Cross-Cutting”. Managers would sell it on being for features like Printing that really are used by many areas of the system, but we all know it becomes a team that gets miscellaneous and unrelated projects. They end up being like the “Service Improvement” team that deals with random bugs, and work no one else wants to do.
Cross-Cutting
There was a meeting where managers were announcing the new Domain Teams and I got assigned to Cross-Cutting. One developer was voicing his concerns about having a Cross-Cutting team. He wanted to point out that Domain Teams are supposed to have specialist knowledge on the Domains but most people that were assigned to their teams had little-to-no knowledge. For some reason he chose my name to make a point.
“What does TimeInInts know about Cross-Cutting?”
Which received a room full of laughter. I’m sure some were laughing at his point, some laughed at his emphasis and delivery, and others probably saw it as an attack on my knowledge. I was probably one of the best people for it really, given my experience in the previous Service Improvement teams.
The whole idea of keeping Domain knowledge in the team only works if there is a true commitment to keep the teams stable over years. However, people will leave the business, some will want to move to a different project to broaden their skills, or people could just fall out with their team members.
Another concern this developer had was with his own team. He was assigned to a Domain team he was the expert on, but was used to working with a couple of developers in the UK. This new team had two Indian developers. They had recently acknowledged the distributed teams weren’t really working so these new Domain teams were supposed to be co-located. But this setup seemed to signal that he was there merely to train these Indians up to then essentially offshore the Domain. Since he was the expert and proud of it, he still wanted to work in that area. But he can’t work on the same software forever.
In the Cross-Cutting team, we had an open slot labelled “new starter” so we were going to get a new hire in. You have to start somewhere, but again, this doesn’t help the teams specialise if they don’t already start with the knowledge.
Colleagues Opinions:
Developer 1:
Me 13:39: what does a new starter know about Cross-Cutting?
Mark 13:39: sounds more like Cost Cutting!
Developer 2:
It’s infinitely harder to build something if you don’t understand the thing you’re building. Hard to catch issues and make sense of designs if you had no opportunity to learn the domain.
Developer 3:
isn’t one of our major issues is we’ve lost domain expertise for core/bread and butter modules. For any “module”, there’s a combination of what the requirements are/how it should work, and what the code is actually doing. Without “domain teams”/ownership – we’ve lost a large part of the puzzle (how module should work).
Developer 4:
our teams are completely ineffective, expertise has been spread too thin. We probably need to reorganise the department again with who is remaining.
Build stronger teams first that only have one junior-ish person, then have weaker teams helping out where possible. It will be very hard for the weaker teams, but unless we do this, we’ll lose the stronger people.
The weaker teams should be given appropriate projects with longer timescales, and given as much help as possible while ultimately having to struggle their own way, stronger people who put in the effort will begin to emerge from those teams.
A few years ago, I was talking to my manager about how we have a lack of people that like to perform Code Reviews, and it needed to change. He said since I had gained a reputation of being a good code reviewer, maybe I should do a presentation. I said it probably wasn’t that easy to come up with something generic and it needs a lot of thought. Maybe I would attempt to write a guide one day. Over the years, I have bookmarked a few blogs, collated some notes and ideas; but never put them together. So here is my attempt.
What Does Code Review Mean?
When a software developer has code they think is ready for Testing, they need to commit these changes to the main branch. Before this happens, there is often an approval process by their peers. This is the Code Review. The Github terminology is Pull Request. I’ve always found that to be a strange name.
Andy: pull requests?! since when do we use that term? Me: when nerds go dating
I suppose you could say the aim is to attempt to perfect the code. But “Perfect” is not defined; does perfect code even exist? The goal is to always write better code.
There’s the 80/20 rule of Project Management which states that 80% of the feature can be written in 20% of the time; but then to perfect it, it then takes the remaining 80% of the time. Sometimes you have to be pragmatic between programmer effort and actually delivering features on time.
Code Reviews are a great opportunity to learn either as a reviewer or the reviewee. Receiving questions, suggestions, or instructions from your team members help you learn about the programming language, design patterns, common practices, alternate ways of approaching problems, or learn about the actual domain. As the reviewer, you are practising critical thinking and essentially debugging in your head which is a skill you will be doing every single day; since more time is spent reading code than writing code.
We all benefit from an extra pair of eyes. Sometimes you think you have written some great code, and keep reading over it and think it is fine. Then another team member glances at it and spots improvements which you agree on. I’ve seen plenty of smart developers write poor code which I know they would flag up if their colleagues wrote it.
Looking at Code Reviews helps team members gain context on other pieces of the code base. Having more awareness of what code is changing gives you a headstart in fixing future bugs when you know exactly where to investigate.
Code Reviews take time, and time is money. Formal reviews are costly; that cost has to be justified by the nature of the software project. Delaying new features and bug fixes can have an impact on users, but also other team members due to higher code churn causes merge issues when it is checked in. The longer code changes stay in a branch, the more likely you run into these merge issues.
Readable code
I have heard some developers say “reading code is way more difficult for me (than writing)”. Given that programming actually involves more reading than writing, then I think developers that struggle to read code need to be questioned!
Since code is read many more times than it is written, you need to write code that is easily read by humans. If we were writing code just for the computer, we’d be writing in binary. Programming languages are meant to communicate to humans first, then computers after that. Although you don’t want to be too verbose, if you are optimising for the fewest lines of code; you are probably optimising the wrong thing.
Tone of your comments
The art in reviewing is to flag all possible problems, but you need to be careful how you word comments because finding problems in someone’s code can be seen as a personal attack, and people can feel like you are questioning their coding abilities. Reviewers can easily come across as condescending and demoralising their team members. The aim of the review is to improve the code, and grow your team member’s skills. It’s not about the reviewer’s ego or a chance to show off.
However, developers should take pride in their work, so attempting to submit some code that took no thought probably deserves a wake-up call. The severity of bad code can differ between systems. Does the software involve the potential loss of life (e.g. a medical device, vehicle safety system) or the handling of millions pounds of assets? Or is it a simple command-line utility that your colleagues use? In these severe situations, I can understand how reviewers lose their patience and write brutal comments. See Brutal PR Comments section.
The reviewer often holds a senior position, but could also be on the same level as the author. Regardless of any power dynamics, you need to bear in mind the author may have way more context and be involved in other meetings about the work; leading them to write the code in the way they did. Given that possibility, it’s better to phrase comments as questions rather than stating something with authority. Instead of “You should do it Y way” you could say “Can you talk about your reasons for choosing X? In most cases I’d use Y, is there a reason not to here?“. This approach comes across as more collaborative and friendly. In the case your suggestion was correct, they should realise when they try to answer. In the case that you are wrong, they can explain the additional information they have; and so all participants can learn.
You don’t always have to be negative as well. You can even add comments to add praise, or to admit that you learned something by reading their code. A bit of positivity helps negate any negative tone you had in previous comments.
Reviewers are empathetic that the recent joiner might not be aware of all the coding guidelines especially ones that are informal ones (not written down anywhere). Reviewers are also well aware that a new joiner is still ramping up with the codebase, and won’t be up-to-date with conventions and functionality.
Where To Start:
Sometimes I just dive straight into the code and start reading. Sometimes this can give you the best judgement to whether the code is clear or not. If it isn’t immediately obvious, or the code looks/”feels” strange, then I will try to gain more context. I read the Title, I read the description provided. I look at the linked Bug/Requirement and read the information that the developer had, and understand the problem they were trying to solve.
To get an overview of the change, you can glance at which files were changed/added/deleted. Sometimes reading the folder names gives you more context. Are you looking at client or server code? Are there database changes? etc. Do the files match up with what you expect; maybe they added a file by mistake. Maybe they added some temporary “hack” they made to test their code and haven’t deleted it.
Read In More Detail
Two questions to keep in mind are
• How could I improve that?
• What could go wrong?
You can review it by two approaches: readability and functionality.
If you can’t understand that code now, in one year you won’t get it either. If code is hard to understand, then it is harder to change, and more error prone. An easy thing to look for are typos, unclear names (“Variables with intent”), ambiguous names, wrongly named code.
Small functions are easy to read, are less likely to need code comments, and also easy to test. You can look for large functions and consider if they can be broken down.
If there are code comments, are they needed? Do they add value? Comments are good to explain hard-to-read code, or weird design decisions the author made because you can’t think of a better solution. Maybe there is a way to make the code more readable without the comments.
Does the code conform to your “coding standards”. An example can be casing eg:
// camelCase
// PascalCase
// snake_case
// kebab-case
Your team may have other rules about styling such as:
“returning early from methods”,
using certain syntax,
Keep argument list small.
However, if a given piece of syntax should never show up in your codebase, you should really add an automatic “linter” rule that will either flag it, or automatically fix it. It’s a waste of time to make this a manual process and it doesn’t provide a ton of value. You could say “if it’s not worth it to add the rule; then it’s probably not worth it to highlight it in the code review either”. Not all things can be linted though such as coming up with good names for variables/methods/classes.
Sometimes, you may have a recommendation that should not prevent the code from moving forward, but you want to note anyway. Marking these things with a prefix such as “NB” or non-blocking can be a great way to flag a small annoyance that the author can ignore if they don’t want to fix now. You might do this if you don’t feel too strongly about the issue, or think it’s not worth holding up the change. You always need to remember to be pragmatic.
A little code review habit I appreciate:
When my teammates make a minor suggestion, they often prefix it with "Nit:"
It's a short, polite way to say "I know this is minor, but I suggest changing this."
Useful for misspellings, typos, naming suggestions, etc
The functionality approach would be considering code for how it meets the requirements, as well as “non-functional” requirements like scalability and security. Is there any code that is redundant or duplicated? Are there any obvious bugs like the classic Null Reference or Index out of bounds? You could also ask yourself “How would I have done it? Is it my way better? Could that logic improve the current one?”
Has the person added any Unit Tests, and if not, can they? If tests have been deleted, is this the correct thing to do?
Does this change impact another system?
Are errors handled correctly?
Is the functionality over-engineered? Are there new third-party dependencies that are unnecessary?
Are they using a design pattern incorrectly?
Does the feature cause problems as the number of users scales? It might work on their machine, but will it work in live?
What should I do when I don’t know the language of the code?
There can be scenarios where you don’t know about a certain coding language or technology, but you have to review it. You can make it clear that you have limited knowledge before making comments. If there is no automatic linting on the code, a good starting point is the superficial review: look for typos and well-name of variables. Try to ask questions so you can learn, and also check their understanding. Sometimes asking questions gets them thinking and they find flaws in their own code.
A related point to make here, is that if someone is writing in a language they aren’t fluent in, they can write against the convention. We had a developer writing C# but was fluent in C++, so we often see him write If statements backwards like: “if (false == value)” which is a c++ naming convention.
“If you’ve ever seen Java code written in a C++ style or vice versa, you’ll know what I mean. I’ve previously referred to this in terms of speaking a language with an accent – you can speak C# with a Java accent just as you can speak French with an English accent. Neither is pleasant.” – Jon Skeet
Approve/With Suggestions/Reject
Once you have written your comments, you can set an overall status of the review. The terms differ depending on the system (ADO/GitHub etc) but it generally follows Approve/With Suggestions/Reject.
It’s possible to select an overall status that doesn’t match your comments.
Andy rejected the pull request The change is implemented perfectly, I’m just thinking we could alter the design slightly to provide better flexibility.
One developer explained his process of what he selects. So he can Approve, but also leave comments. But that is a different message from when he uses the “With Suggestions” and leaves comments.
The way I do code reviews is as follows · Just comments – I’m starting a conversation on the change, and will Finish it later, usually I am expecting you to reply, so feel free to reply to my comments. I usually choose a “finish reason” after discussion. · “Looks Good” – just check it in. · “Looks Good” + Comments – just check it in but I had something to say. · “With Comments” + Comments – there are minor things, style/formatting that I'd like changing, please make where appropriate (I can be wrong) and check in. I don't need another review. · “With comments” + No comments – I am agreeing with someone else’s comments, or if I was first, I probably clicked the wrong button - check with me for clarification. · “Needs work” + Comments – Please make the suggested changes and submit a new Code Review. · “Needs work” + No comments - Agreeing with someone else’s comments, or if I was first, I probably clicked the wrong button - check with me for clarification.
Brutal PR Comments
John If you want something done, do it yourself. Yesterday at 10:06
John Well, this shows that you did not even attempt to OPEN the DB Utility Tool, let alone test via the DB Utility Tool. It would crash opening this. Line 351 | 5 hours ago
John I have not reviewed any of the C# code, I expect it to be as depressing as the DB code though. 5 hours ago
John What the hell is this doing in here? Also why have you stopped inserting the ONE important thing from this trigger - the change to the organisation! Line 101 | 5 hours ago
When To Do A Project Review
When it comes to merging an entire project, this can consist of hundreds of files. Despite being reviewed by team members, the final merge will be reviewed by an Expert. We have tried to get Experts involved early in the projects but since it can take a long time and the deadline can be far away, they aren’t inclined to do it. Then when you want to release it in the next few weeks, they will review it and dump loads of comments on it, blocking the merge.
“This is taking a long time and there are quite a few problems with it, nothing that can’t be fixed in a week or so, but some things that should have been flagged up by someone / thing before it gets to me. This process has to change.” – Expert
You probably need to ensure that each team has an Expert Reviewer in the team, so that quality reviews are done throughout the project. We often didn’t have teams structured in this way.
“they need to stop having teams that don’t have at least one person who knows what they’re doing” – Dan
One of my colleagues wrote the following about this issue. He often gets blamed for holding up projects when he is being asked to review with limited time. Any feedback he gives then blocks the project, and if he doesn’t review in time, then he also blocks the project:
Mike’s Pull Request (PR) ideas
For the most part we are doing a good job with pull requests, but occasionally I feel we can do better. I’ve thought of some useful guidelines, that will ensure quality, again most people are following these, so great job, but please keep them in mind PR Guidelines
Your team should be self-sustaining:
As a developer you should always be able to send your PR to someone in your team for a thorough review.
If you’re regularly having to pull in external resource to review your code, you should make your team leader/scrum-master aware, so they can discuss this with resource managers.
Code should always be reviewed internally before someone external to the team is added to the review, this ensures that the external reviewer only sees code which has survived the initial review pass.
If external expertise is required:
Let your team leader/scrum-master know that expertise is required, identify the person with expertise and contact them to let them know you will require their time, preferably a sprint in advance, so they can discuss with their team and prioritise time in the next sprint.
Your PR is not “urgent” unless its SLA is at risk of expiring.
You are not to refer to external reviewers as “a blocker”. If external expertise is required, then it is an essential part of the development process, and they are only seen as blockers due to poor planning.
Draft PRs are not very useful to external reviewers, since you can only comment on them: not approve, but they’re great for sharing day-to-day progress updates between remote developers.
They should be used to update your team’s senior developers and technical architects on your progress, and receive feedback.
I would say in a well-oiled team, that developers should share code each day, by some mechanism that makes it visible to their seniors for feedback, this ensures valuable short feedback-cycles and is the most cost-effective way of ensuring quality during development
Respect the reviewer
I think a key takeaway from this idea is that you need to respect the reviewer. They are kindly dedicating their time. You also need to understand that the review process is there to improve code quality and not just a box-ticking exercise.
I find that sometimes people create a review, then instantly message you about it – even though you are often notified through alerts you have set up, or will check your review list when you have free time. Being nagged is not nice.
There have been times where I have submitted comments then am messaged a few minutes later asking to re-review. If you ask that quickly, then I know that you didn’t even build your changes never-mind test them to see if they work. Should I really approve something you took no care with? (Maybe a 100% unit tested solution would mean that this is possible though).
We also usually have a rule that 2 people need to review, so even if I approve it; then it still cannot go in, so I hate being nagged to approve when there is time. Sometimes code needs more thought to consider if there’s more aspects to it than initially thought. A rushed review isn’t a quality review.
Making statements like “please approve this, it needs to be released tomorrow” isn’t good for the reviewer. I want to be able to review it properly, leave comments as I wish, and even Reject it if I really don’t think it will work.
Conclusion
If you see reviews as just a box-ticking exercise, then it defies the whole point of the review. It really needs buy-in throughout the team. If you want quality and continuous improvement, then support the review process. If you want speed, then it can be sacrificed at the expense of quality, and other benefits.
The code review process has a wide range of benefits and outcomes: teams see improved code quality, increased knowledge transfer within and across teams, more significant opportunities for collaboration and mentorship, and improved solutions to problems.
Many years ago, when I was a Software Tester, I remember when we had to write a Test Specification based on the work that the Developers had planned. This was for both Enhancements and Bug Fixes (so new features and changes to old ones).
It would be a Word document, with the Item Number, Title, and then a description of what you would test (the description would be a bit more high level than the step-by-step description featured in an actual Test Case).
You would spend weeks writing it, then you had to get it approved by all the developers, or the Dev Lead. The developer often then told you it’s nothing like you imagined so you had to rewrite it.
Sometimes they would demo the feature to you so you had a better idea. If they had no comments, I often felt that they didn’t read it.
When there was a new Developer who wasn’t familiar with the process, he “rejected” a Test Specification because of some grammatical issues. I think it was something to do with the wrong tense, and not using semicolons. The Tester was fuming, but he was quite a belligerent character.
I think we often worked from the Bug description, or some comments from the Developer. However, quite often, the comment section would be the Developer writing something generic like “test bug is fixed”, “check data is saved“. If it was more detailed, sometimes you would paste the developer’s notes and change a few words – and have no idea what it meant until you saw the finished thing.
The Verdict
I think both Developers and Testers saw Test Specifications as a waste of time. The Developers weren’t enthused to read it, especially when most people rewrote what the Developer provided, and that might not be the full test coverage needed. The Testers should be adding more value by using their existing knowledge of the system to come up with additional scenarios to cover “regression testing”.
I think the only advantage is to quickly verify that the Developers and Testers were on the “same page”, but that only works if the Tester has not used the developers words and tried to illustrate that they do understand the upcoming features.
I think it eventually got binned off for what we called “Cycle Zero Testing” which was where the developer quickly demoed their changes; which I think was still hated by the Developers but was easier to understand the value, and was a more collaborative between the two roles.
Some teams post fortnightly updates on what they have done on our internal social media platform Microsoft’s Yammer (Now named Viva Engage). No matter what they post, managers seem to respond with positive comments.
So here is an example. On team stated they had completed:
3 user stories. 1 dev investigation. Improved code coverage.
Then attached was there code analysis report which stated:
0 Bugs
0 Code smells
100% coverage on 1 new lines to cover
0% duplications on 8 new lines
I never know how to make sense of these stats. The duplications mention 8 new lines, but yet the code coverage mentions 1 new line. It can’t mean it is still to cover when it states they have 100% coverage.
Their video demo stated it was “Sprint 8” which means it’s after 16 weeks of work. They logged into their application and there are 2 buttons (not functional).
My Work
I’ve been working on a game in Unity and have also reached the 16 week mark. I have only been working on it sporadically. Early on, I might have worked a few hours and many days per week, sometimes working several hours at a weekend. Other weeks, I have just worked on it for 3 hours or so in one afternoon only.
I have written loads of code for the AI state machine. Your units can automatically move in a formation, break away in certain conditions. Then the defending group of units have their own behaviours, and work together in their own way. Some behaviour is configurable so I have the logic and UI for the player to change it. There’s controls for the cameras, game speed, pausing, save and reloading. A few options for volume and a few graphical effects. There’s some miscellaneous aesthetics such as animations, particle effects.
I am a single developer working fewer hours, and I have to play-test it myself. Compared to a team with a few developers, a few testers and different managers assigned – all working a 9-5 day, 5 days a week; and all they have is a menu and a title banner.
Hackathon Progress
Another comparison is to compare the progress of developers during a “hackathon”, or “game jam”. Developers can often put together a prototype, or a fairly fun game within a few days…
Developers during hackathon: We built an entire application in just 3 days.
Developers after hackathon: Adding that icon is going to take 3 weeks.
Developers during hackathon: We built an entire application in just 3 days. Developers after hackathon: Adding that icon is going to take 3 weeks.
Mark Dalgleish
Conclusion
Recently, I’ve been thinking about this a lot. I think if you start with no code, then any code you write has a much bigger impact. You also don’t have to work out how it works alongside other code.
So the more code there is already, then the longer it takes to write. So if the Hackathons are productive, then you can write something fast. Same situation with my Unity game, and since I am working on it myself, then I have no one slowing me down.
The other reason why working on my own is great is that I can set the process. Due to the 90/10 rule, I think it is best just to get it 90% done, then move on. If you change your mind of how something works, it isn’t as big of a time waste. If it turns out to be good, then you improve it later, alongside other features when you are changing code in that area.
So going back to this original team who did 16 weeks work and got nowhere – even though it’s a new software product, and should be able to code fast – I think they are slowed down by their insistence that everything has to be perfect and has all the extras like Unit Tests. So they could get a feature 90% complete quickly, but then they are told to spend several days perfecting it for that last 10% of quality. Then they are messing around with non-functional stuff like the Deployment Pipelines and miscellaneous processes. When you are months away from releasing, I don’t see the point in dedicating so much time to how you are going to deploy it, so I have been 100% focussed on features and gameplay.
I think this could illustrate how process can severely slow you down, and striving for perfection can also lead to inefficiencies. I think my idea of “90% quality is good enough” could be a winning approach, then you can always perfect the features if you are ahead of schedule. If I keep working on my game, I will worry about how to release it when it is close to a releasable state (ie how to get your game listed on Steam).
When a new development team is created, managers insist on creating a “Ways of Working” which essentially comes down to describing the software development and testing process in the ideal way.
Then after a few weeks, you end up just resorting to the default way of working.
The “ideal” way assumes that the Testers can provide ideas and inspiration to the Developer, and when the Developer writes the fix, the solution won’t deviate from their original perception. Therefore, all the Test Cases are written up front, and none are needed to be written after.
Here is an example that one of my former teams came up with:
Before fixing the defect
Analyse together
Make sure you pair up as a developer and tester to work on the defect.
Review the problem together
Understand how to replicate the fault
Understand the extent of the problem (all regions, all set-ups, all environments?)
Do the initial debugging to understand where the problem lies
Fixing the defect
The developer MUST
Make sure there are unit tests to support the current functionality
If there aren’t unit tests then implement a simple refactor which will allow appropriate unit tests to be added
Add unit tests that prove the defect (which will fail)
Fix the defect
Get it code reviewed
If you are working on code you are not familiar with, get 2 reviews, one of which must be from an expert
Run all the unit tests and prove the fix
The tester MUST
• Write test cases with steps and expected results for the initial test cases. They must
○ Be clear and unambiguous
○ Cover the different scenarios/circumstances/options
○ Not test things that are irrelevant
• Once written, set the state to “Review” and assign to the reviewer
• The reviewer
○ Adds comments as appropriate
○ Set the state to “Ready” if Ok, or back to “Design” if it needs changes
○ Assign it back the test author
• Only use Given, When, Then if you are using Specflow to automate your tests
• Only use exploratory testing
○ If you have passed a Rapid Testing course
○ And you have a clear understanding of testing heuristics
○ And you plan and classify sessions, keep session logs, run debriefs
○ And all the specific test cases and all regression test cases have been executed
• Create regression tests of the functionality
• Create test case(s) to prove the defect
• Get the tests reviewed
• If you are working on functionality you are not familiar with, get 2 reviews, one must be from an expert
Assure the Quality
Review the actual changes together
Understand the logic
Understand the scope
Minor/contained (inside a method that’s inside a class that doesn’t affect anything that’s outside it)
Medium/impacting (a change that impacts outside it’s own class or is called from elsewhere in the system
Major/widespread (a complete refactoring of existing code or large changes to core features)
If they don’t understand the logic or can’t establish the scope, escalate to the Technical Manager
Execute the tests
Attach and link all work items and evidence together
I found loads of chat logs from work, and additionally found a few quotes I wrote down from various employees. So today we are going to discuss the legendary employee Steve.
“Steve looks like a confused garden gnome that lost his hat”
Adam – colleague
Introduction
Steve joined as a Software Developer, and I think Steve’s carefree attitude meant his code was a bit inconsistent in quality. I liked working with Steve though, he was often quiet and just got on with his work, not really paying attention to anything else. It’s a pro and con really. Work got done (though often you had to prompt him to tidy parts up, or spot mistakes for him to perfect it), but then he didn’t know who many employees were because of the lack of attention around him. When he joined in the banter, he was quite “laddish”. A simple northern lad, Steve was well known to like his food and beer.
I just looked on his Facebook – and under “Political Views” it says “Democratic Alliance for the Betterment of Hong Kong.”
Andy
Sometimes he seemed to be quite unlucky and trivial events became more hilarious. Once, Steve opened up the window to let a butterfly out, and another one came in.
He always wore a t-shirt and even when the aircon was chilly, he loved opening the window. Sometimes even had the fan on whilst sat next to the open window.
Me 13:13:
how come Steve has a different body temperature to everyone else? Sometimes I think he isn't human
Andy 13:13:
haha, has he got his fan on?
Me 13:14:
he has the window open. Last week the air was directed at us so we were freezing. Now we have got him to open a different window, it's not so bad. Although, after a while, it does get freezing but Steve insists he isn't cold. Meanwhile Liam just said "I'm wearing my headset to keep my ears warm"
Knowledge of Colleagues
“I know people that are relevant to me”
Steve
Matt: "Do you remember Colin?" Steve: "No, of course I don't"
“out of all the companies I’ve worked at before, I can only remember about 2 names”
Steve
Me: "Why is Simon leaving?"
Steve: "I don't care"
After our team member Paula moved up to Scotland and worked in our office there, Steve asked if Paula “was on her own, or if there was someone else on her team up there“. Paula is on our team, therefore if someone else was on her team, they would be on our team.
Charlotte asked Steve who wanted all these database changes and he said “John Bundy”. There is no colleague called John Bundy, and there never has been.
Matt: "The documents work item needs moving off the board because the Documents team are doing it"
Steve: "who is doing that? is it Gary?"
Matt: "No, it's Tony. You emailed him about it last week"
Steve: "Oh yeah, I did"
In his update, Steve once said “A chap called Jon Reaves has made some changes”. Jon had worked there for several years and is well known to everyone. Saying “a chap called” suggests he had never heard of him and thought he was new.
Food
Having 2 glasses of wine a week is unhealthy. You should be aiming for 30 units a week, mainly from beer.
Steve
“I’d rather eat my own feet than a KFC”
Steve
“Giving up beer and pizza is never a good idea”
Steve
Tracey was explaining how she went to London and had a fancy meal in Gordon Ramsey‘s restaurant. Steve chimes in:
“I went to Sheffield and had a kebab”
Steve
Matt: "I tried loads of stuff in Vietnam, no idea what it was"
Steve:[loud and affirmatively] "Bollocks"
It could have been testicles, Matt was explaining the interesting and different meals they have there, but it was funnier the way he said it like he had no doubt it was that.
“Four pints is what I call breakfast”
Steve
Steve was complaining that the office canteen has had “Toad in the Hole” for 2 days running. I said “I bet you ate it anyway”. Then he replies in a passive-aggressive tone:
“what else am I gonna do? eat the vegetarian option? Not likely.”
Steve
We once had 2 offices located close together. Our team had moved to the other office but we received a mass email from Mark stating he had brought cake in and placed it in the kitchen. Steve started walking to our kitchen (in the different office), Matt told him it’s not in that kitchen… but Steve checked anyway! He was desperate for that cake.
“Chickens come from seed which comes from oil”
Steve
Matt was originally talking about cars. Then Steve said all food comes from oil, then said that. I was instantly lost.
Software Development/Attitude to Work
“Matt! Myself and Phil are having a bit of a disagreement, and it’s about to turn to blows”
Steve
Matt: "Steve, have you done your Information Governance training?" Steve: "I did it last year" Matt: "what does the email say?" Steve: "It said it is fine" Matt: "Read it again"
In our team “Retrospective” meeting, we had to vote for “Team Member of the Sprint”. Steve voted for me. Matt asked him for the reason and he said
“I was hoping there wasn’t a second round of questioning”
Steve
A few weeks after finishing the Online Request project:
“Do you know how to switch on ‘Online Requests’?”
Steve
Me 13:41 guess how many unread emails Steve has. It's like he has been on holiday for weeks Dan 13:42: 100 Me 13:42: way higher Dan 13:42: 500 Me 13:42: closer, higher! Dan 13:42: I give up Me 13:43: 550
No wonder he didn’t know what was going on.
His manager, Matt once stood at his desk and simply stated “Steve”, and Steve was baffled. I correctly assumed it was his one-to-one meeting. Even after Matt told him to check his calendar, Steve was still baffled what it could be. Classic Steve. Probably a meeting request in one of his unread emails.
We once had a meeting located in the main office. All our team dialled in remotely apart from Steve. From the video feed, we saw him walk into the meeting room late and say something to Adam.
Andy 12:36:
did you see Steve randomly turn up to the meeting?
funny as can be
Me 12:36:
yeah
Andy 12:36:
Mia had tears streaming down her face
Me 12:36:
why?
Andy 12:36:
cos why did he turn up when everyone else on his team dialled in
Me 12:37:
did he ask Adam if he was at the right meeting?
Andy12:37:
yeah!
“I was thinking of going for ‘Looks Good’ because there’s too many files”
Steve on doing Code Reviews. Too many files gets instant approval.
Charlotte: "what did everyone think of the meeting yesterday?"
Steve: "What meeting?"
Charlotte: "the meeting with Ronnie"
Steve: "oh, that. I'll be honest with you. I wasn't listening. I have no idea what was said"
Steve took an extended lunch break, and then later he went for a long walk. Matt challenged him on it "Didn't you go out for lunch as well?"
Steve said "yes" with a right cheesy grin
Doesn't care.
“Soon, I’m gonna be introducing lots of bugs. I’ve nearly finished my work; and I’m not dev-testing it”
Steve
Dan 16:18:
is he… what!? is he trying to get fired in the same way you'd act like a jerk to encourage your partner to split up so you get to feel morally superior?
Me 16:18:
haha, great example
A similar example…
Matt: "Steve, are you sure these changes haven't broken anything?"
Me (with fake confidence): "Yeah, because he ran the unit tests"
Steve: "Have I? I only ran the build"
Steve wrote a unit test with the following test data (Michael Jackson).
Me 13:53:
user.Surname = "O'Cake";
user.GivenName = "Pat";
Andy M13:53:
i'm sure i've seen that before
Me 13:54:
reminds you of your days in Pre-school
singing children's songs
Me 14:14:
OMG STEVE IS HILARIOUS
Matt googled Pat O'Cake and its a character from Bottom. He asked Steve about it, and he said "I wouldn't Google them all though, sometimes I use pornstar names"
Then a week later:
private const string _vouchingUser = "Bearstrangler McGee";
Andy 10:52:
wtf
Me 10:52:
Steve special
I never dare search for anything Steve puts in unit tests after he said "I sometimes use porn star names"
Andy 10:54:
haha
i hope that's not the name of porn star
Then there were some interesting reasons:
Me 14:48: "CancellationReason\": \"patient has lost keys to handcuffs\" why is Steve different? Andy 14:48: what the hell? Me1 4:48: const string cancellationReason = "patient was visiting a massage parlour"; Andy 14:49: is he checking this stuff in?! Me14:49: yes, it's in our branch Andy14:50: he's an absolute lunatic
Steve was working on fixing a bug that Matt was also fixing (but we didn’t know it at the time). The next day Matt and Steve were both on annual leave, so Matt had handed his work over to me, and Steve handed his over to Jim. I finished my work, and Jim even passed my code review without even realising the similarity. It’s like a comedy show sometimes.
Steve had completed a feature, but his changes had broken Matt’s last bug fix.
“it worked for my user story”
Steve. It’s like the classic “it worked on my machine” that software developers love to say
Steve completed the work for saving Users to the database. I just tried it and it crashed. We asked him how much testing he did and he claimed it was all working. I showed him and he said “I forgot about that way”. There are only two scenarios, add from existing user, and add new user.
“I don’t think the Database Tool is working. I think it is completely goosed”
Steve
I just caught Steve smurf naming even though in his last code review, Phil told him not to. So then he looks up Smurfs on wikipedia. He clicks Smurfette and says “I’ll see if she is fit“.
I have no idea who brought a “dunce hat” in, but we decided that if you somehow break the build, then you wear the hat. Steve wore the hat quite a bit.
“I don’t need to wear the hat; I haven’t broken the build. I’ve just broken the product”
Steve
Not sure how he did it, but Steve once sent code to review which had the same title as the previous change he did. It also had the wrong User Story linked to it. (－‸ლ)
I told Steve that he was supposed to roll back one of his work items. After a few seconds he said it was done. I was sceptical. He said that I had already deleted the other part of the change. So I looked, and I hadn’t. He then said
“to be honest, I didn’t even look at it. I didn’t even compile it”
Steve
Miscellaneous
“Any advice that starts with ‘do not expose’ is good advice”
Steve
Liam was telling Steve that an angry resident left him a note on his car telling him not to park there again. Steve then comes out with this…
“Just piss through their letterbox”
Steve
We were playing badminton after work, and Steve said he had to rush off. Mike asked “are you doing something interesting?”. He said his parents were coming over later and he had a massive stash of weed to hide or smoke.
“I accidentally googled porn with my mum on mother’s day”
Steve
He was helping her with a crossword and the clue was “goddess of nature” and he wrote “goddess of mature“
This post outlines a useful structure for product-based software engineering teams. I believe it is a fairly standard structure in a modern software company. I’ve adapted this post from a contribution from a colleague.
The main principles are:
to keep communication flowing;
accountability is defined and understood;
the ability to scale horizontally
People
A team size should be 7 +/- 3, or as Jeff Bezos calls it “The two-pizza size rule”: If you can’t feed the team on two pizzas, the team is too big.
The composition of the team should involve these people.
Technical Lead
Senior Engineers
Engineers
Junior Engineers (or Graduates/Apprentices)
A trend in the industry is to move towards the general term of “Engineer” rather than distinguishing between Developers and Testers. However, if there’s a lack of Automated Testing, then more Manual Testing is required which then has a stronger requirement to make this distinction between the roles. Having a distinction can lead to the “us and them” culture as code is “thrown over the fence” to be tested. The team can feel more fragmented – this isn’t always the case though.
Technical Leads do not necessarily need to be the most technical on the team, but they need leadership and management skills. They need to know who can solve an issue in their team and be the facilitator.
Juniors (or graduates/apprentices) are an important part of growing your business. When many job adverts ask for the standard “2 years experience”, how do you get it? Recruit juniors into your team and teach/mentor them, shaping them into the engineers you need.
A team needs to be a blend of skills, strengths, personalities. A junior engineer is a part of that blend. It keeps the senior members in the team on their toes, as they need to coach, mentor, and explain concepts to receptive minds. This can ground the team, and make them more productive.
Earlier we defined people, not roles. These are the roles that augment the team:
Product owner
Architect
Scrum master
These roles could be people in the team, but not every organisation will be at the scale to require a Full Time position for each team. If you don’t have a dedicated full time position, the team will need to decide who will pick up the duties of these roles.
Accountability
Responsibility – Who feels guilty when the team doesn’t deliver, or something goes wrong.
Accountability – Who takes the blame when the team doesn’t deliver, or something goes wrong.
When a team is formed, they are all responsible for everything the team needs to do. As a member of the team, you do whatever is needed to “get over the line”. Your process should not create single points of failure, or knowledge silos. Your process should provide mentoring to junior team members, and be supportive of learning.
The Technical Lead is accountable for making sure everything is complete and to the desired standard.
So what is everything? A non-exhaustive list would include:
Technical/Solution design.
Backlog management.
Both product and technical.
Development and testing.
Deploy and release management.
Support and documentation.
Monitoring and telemetry.
Reporting to managers
Sprint reports.
Roadmaps.
Quality metrics.
Quality control.
Line management can be with the Technical Lead, split between Technical Lead and another Senior within the team, or even people outside the team. If it is the latter, then the accountability needs to be clear.
Scaling
Scaling needs to be “horizontal” when a team unit becomes saturated: You do not make teams incrementally larger to scale your output. You add more teams and give accountability to these new teams. This goes back to the 7 +/- 3 idea.
you need to add another team:
If the average velocity of the team is not delivering the quantity you require.
If you are asking the Technical Lead if they need more people to deliver, and they are already at the top end of the 7 +/- 3 bracket.
If you are scaling, you are likely to come across engineering teams that need to work across product teams. Examples potentially could be: security; authentication; UI Libraries; or overall platform stability/accountability. Whilst these are individual products in their own right, they are servicing other development teams. They need to be more collaborative, open, and aware of the business needs of other teams. Their decisions will impact more people than decisions made in a single focus team. They will need to be honest and clearly communicate priorities.
There’s no point in pasting the entire book in my blog, so I’ll just give the initial paragraph to highlight their ideas.
They are mostly negative ideas, or can be in certain situations. However, there are some positive ones such as “Bullseye Commits”.
PART 1 Work patterns exhibited on an individual level
1. Domain champion
The Domain Champion is an expert in a particular area of the codebase. They know nearly everything there is to know about their domain: every class, every method, every algorithm and pattern.
2. Hoarding the Code
This pattern refers to the work behavior of repeatedly working privately and hoarding all work in progress to deliver one giant pull request at the end of the sprint.
3. Unusually High Churn
Churn is a natural and healthy part of the development process and varies from project to project. However, Unusually High Churn is often an early indicator that a team or a person may be struggling with an assignment.
4. Bullseye Commits
This pattern is relatively common in most teams, but it often goes unrecognized: an engineer understands a problem, breaks down the project into smaller tasks, and submits code that has little room for improvement.
5. Heroing
Right before a release, the “Hero” finds some critical defect and makes a diving catch to save the day. More formally, Heroing is the reoccurring tendency to fix other people’s work at the last minute.
6. Over Helping
Collaboration among teammates is a natural and expected part of the development process. Over Helping is the pattern whereby one developer spends unnatural amounts of time helping another developer to get their work across the line.
7. Clean As You Go
A codebase is continuously evolving by nature, but it doesn’t evolve evenly across all aspects. A Clean As You Go engineer will notice and refine shortcomings even if it’s not essential to the task at hand.
8. In the Zone
This pattern is exhibited by engineers whose work is, in a word, consistent. They have a knack for getting in the zone and shipping high quality work week in and week out. Their work is reliable and predictable in nearly every way.
9. Bit Twiddling
Bit Twiddling is like working on jigsaw puzzle to the point where everything looks the same and you’re not making progress anymore. You might pick up the same piece, try it in a few places, rotate it, put down, only to pick it up a few minutes later.
10. The Busy Body
The Busy Body is an engineer who skips all over the codebase: they’ll fix a front-end problem here, jump to some refactoring, then fiddle with the database over there.
PART 2 Work patterns exhibited on a team-wide level
11. Scope Creep
Intuitively, we all know what Scope Creep is — along with its associated risks. Still, there are plenty of different definitions for the issue so here’s what we’re focusing on:
Scope Creep (noun): a pattern whereby the originally agreed upon scope increases or changes after implementation has begun. Often, though not always, Scope Creep happens incrementally and thus invisibly
12. Flaky Product Ownership
Miscommunications between Product and Engineering can easily lead to Scope Creep. Flaky Product Ownership, however, can show up slightly different in the data and also generally requires a different approach
13. Expanding Refactor
Expanding refactors happen when a planned effort to improve or revise a section of code triggers a dramatic widening of scope.
14. Just One More Thing
“Just One More Thing” refers to the pattern of late-arriving pull requests. A team submits work, but then—right before the deadline—they jump in and make additions to that work.
15. Rubber Stamping
Rubber Stamping is the process by which an engineer approves their colleague’s PR without giving it a substantial review.
16. Knowledge Silos
Knowledge Silos are usually experienced between departments in traditional organizational structures, but they also form within teams when information is not passing freely between individuals.
17. Self-Merging PRs
This pattern refers to when an engineer opens a pull request and then approves it themselves. This means no one else reviewed the work and it’s headed to production!
18. Long-Running PRs
Long-running pull requests are PRs that have been open for a very long time (more than a week). A PR that doesn’t close in a normal amount of time (within a day) can indicate uncertainty or disagreement about the code.
19. A High Bus Factor
“Bus factor” is a thought experiment that asks what the consequence would be if an individual team member were hit by a bus.
20. Sprint Retrospectives
Retrospectives are a common practice that offer an easy way to continuously improve: take time to reflect, as an individual or a team, on a project, action, or occurrence
My Thoughts
There’s some good observations here, and the book does go into how to spot these aspects and what you can do to counter the negatives.
Where I work, we used to focus on having “Domain Experts” which is the “Domain Champion” idea. In recent years, I think managers have drastically swung the other way which leads to “The Busy Body” who knows a bit about many things but doesn’t specialise; so now we have the opposite problem.
There are a few areas where we still have a “Domain Champion” and they tend to get work related to that area if they like it or not. Even when it does get initially assigned to someone else, they take too long and then say “the deadline is approaching, I think the Domain Champion should do it”.
I always complain we don’t have many developers that are willing to do Code Reviews, or at least; do them properly, so you end up getting “Rubber Stamping” which is like having no review at all.
I personally like to “Clean As You Go”. It’s not a good idea to go far out of your way to change code that didn’t need to be changed. Doing so can dramatically increase the risk of introducing a bug and is “Scope Creep” since more areas now need to be tested. I do some “Heroing” from time to time, mainly because I do the most Code Reviews so I spot more opportunities to improve things, or point out bugs in their code.
Recently, with people quitting or moving teams, the Bus Factor has been severely lowered. Currently, I am the only Developer on the team, so if I take time off; then no work at all is getting done.
Several months ago, new teams were formed and we were shown the proposed structure. I thought there were a few strange decisions.
When there’s a team of 8 people, what’s the chance that you would end up with 2 James’ and 2 Louises?
I swear managers like to go out of their way to be annoying
It’s almost like they think “how can we maximise the communication problems?“
If I was choosing teams, I would notice this, and would swap teams to remedy this. I can understand it might not be possible due to job roles and skill sets, but I’m sure most scenarios could be avoided. I’m sure these teams were chosen randomly rather than having any rationale.
Another team had Jack and Jacqueline. I pointed this out to Jack and he didn’t think much of it… until some members of the team referred to Jacqueline as “Jack” and the conversation became confusing.
The team that I recently moved to involves a third party API which is locked down to only allow network traffic from the UK. (I don’t know why the test API has these extreme restrictions – probably unnecessary “red tape”). It should be obvious to managers that this team needs to be fully UK based, but their insistence of being inclusive meant they still added members located in India.
I was replacing a UK developer who left because he was asked to do everything – because the Indians couldn’t connect to this API. In the first week when I joined the team, I was busy working, when one of the Indians contacted me and told me to pick up a bug fix because it was high priority, and they wouldn’t be able to debug/fix the bug accurately/comfortably.
We had 2 fairly urgent bugs and I was being asked to do both of them, yet I didn’t know what the 2 Indian developers were doing in the meantime. No wonder the other guy left – the team’s composition is nonsensical, and the Indians are always going to use this excuse. I’m sure the Indians don’t actually want to be in the team though, they would be more comfortable being assigned to any other team.
You would think creating teams is generally easy, but I suppose there’s a bit of complexity to it. I’ve never had the opportunity to propose a team structure. I’d probably say my method would be something like this:
Pick the correct number of Developers and Testers, considering their ability and knowledge. Do they have experience in the areas the team is focussing on? Does their job title really reflect their true ability? No point just putting one Senior in the team if they don’t have Senior qualities.
Consider their personality types, are there any leaders in the team? No point putting together loads of silent types. Conversely, you probably don’t want too many big personalities.
Consider if there’s any problems due to their location (access to networks or physical locations, consider time differences).
Consider how well people get on:
If you have people who are really close and are productive, then it’s most likely a good idea to keep them together.
Conversely, people that have caused conflicts in the past could be placed in separate teams. No point risking extra tension.
Try and avoid having similar names. This gets confusing in both written and spoken communication.
You may be limited when people can start working in the new team. Presumably everyone is already working on something and will need to transition.
Managers often talk about “allocating resources” when discussing project teams. I don’t understand why we are dehumanizing people. Why can’t we just say people/staff/developers? If we are talking about money or hardware, then it’s fine to use the term “resources”. However, the term seems to be embedded in the business culture since the department that deals with people is often known as “Human Resources (HR)”.
I was watching all the Bourne films recently, and in the scenes where you see staff in the agency office who are monitoring security monitors – they say lines like “Asset is on the move”. It probably makes sense here to dehumanise people; because your end goal is to kill them, so calling them “assets” and using terms like “dispose”/”eliminate” probably removes you from the fact that this is a person who has feelings and a family, and you’re about to end them. Maybe managers talk about people in this way so it’s easier when it comes to redundancies. “Cut 10% of the resources, and move on”.
I think it isn’t even that effective to talk about people using their job roles. When managers only look at the Job Titles and not the individual skills, then they end up creating imbalanced teams. This could be that you need to spread a certain skill across teams such as SQL Databases, so a manager who only looks at Job Titles could end up putting the people with SQL skills together.
Additionally, some Seniors aren’t that great, and therefore lower-ranking Developers are better than them. I’ve definitely seen it happen where managers create a team consisting of 3 underperforming seniors, then wonder why it isn’t working. This was a source of my frustration a few years back and was some major motivation to start this blog.
Creating a team based on 3 underperforming developers is a rarity, but a recent trend for us is to have only 1 Senior leading a few Developers and a Junior. If the Senior isn’t very good, then the team has no guidance at all.
In conclusion, I think managers should show respect to staff and refer to them as people (not “resources”). They should also try to understand where individual people’s skills are, rather than simply making assumptions based on a Job Title. This should lead to better balanced teams. Balanced teams should lead to high performance and morale.
Stranger Coding Tings 