A story about a feature gone bad

Chad clicked the button and created his pull request. He had worked on his feature really long — it must have been three weeks. Finally, it was time to integrate his changes into the master branch so they could go live with the new version of the app. This was the first huge feature that was his responsibility. His team lead, Janet, gave him the ticket for the task and he set out to write his code. "When it finally goes live," he thought, "the churn of our users should drastically go down!".

Primarily he had found many places in the app, where users committed some actions. Up until now, these actions weren’t registered anywhere, so nothing and nobody tracked them. There was no record of what the user did or didn’t do inside the app. Marketing alerted management that too many users did not renew their accounts, or outright canceled. In turn, management asked the developers to do something about it. Together the team decided to record all user actions and put them into a log of all activities for this user. This way they could make calculations which users were not active in the app and reach out to them to prevent them from abandoning the application, or so they hoped.
The development of the feature enabled Chad to take a thorough look at the whole application. After all, he had to integrate his code into all kinds of places. And so he did. Today he was finally ready to publish the code and begin the merging. To integrate it into the master branch so that it could get deployed, he had to create a pull request. "Since my code is well written and worked when I tested it, it shouldn’t take too long for this merge to complete." was his conviction. He assigned the pull request to his team lead and another backend developer that he had talked to during lunch a few days earlier. Chad used the lunch to tell her about his progress on the feature, and she seemed interested. So another set of eyes shouldn’t hurt.

When Sarah, the backend developer on the team, started work the next day, she saw the notification for the pull request in her email inbox. She decided to take a look. After all, Sarah didn’t like it herself when pull requests lingered for too long. So she dove right in. The first thing she noticed was the warning git gave her: The pull request couldn’t be merged because it had conflicts with master. She decided this should be something minor because it happened rather often and decided to ignore it. Chad would just do a quick rebase, and everything should be fine. No need to worry.
She proceeded to look at the changes: 4100 lines were added, and 2521 were removed. There were many files changed as well because Chad copied his code into all the different places. When she looked into the data, things looked strange because the code in all these different parts of the app didn’t look the same. Sometimes Chad’s code style fit with his surroundings, but most of the time it didn’t. At least his code looked the same everywhere — because he made sure to pay attention when copy-and-pasting his changes, he later said proudly.

Anyway, Sarah didn’t want to take too long for this review. Since Chad’s code looked the same everywhere, she reviewed two files and what she saw made sense to her. Sarah remarked that it was somehow hard to follow what was happening because he didn’t follow her style in the parts that she wrote. Especially the variable names, she didn’t like. But she would give his pull request a thumbs-up if he could change them in her part of the code.

Janet took a look during the evening the next day since she had lots of meetings to attend. The first thing she did, was asking Chad what this pull request was all about. He hadn’t written any description for his pull request. She also remarked that the conflicts with master were due to this feature taking so long. Many parts of the app had already evolved from his weeks’ old state. There was a lot of work to update everything. She also wanted him to proceed quickly with this since this feature already took too long. Marketing was eager to get something into their hands since users left the app left and right.

It took Chad another two days to update his code and bring everything in line with the changes from master. Eventually, the conflicts were gone and Sarah and Janet thumbs-upped the pull request to be merged with master.
It went live after weeks of development with only a very superficial review.
Marketing began to receive logs of who was inactive in the app, and where active users spend most of their time. They also received angry calls from customers that the app is slower to respond than usual and that it sometimes just crashes—in places that used to work before.

It turned out that Chad’s changes were not tested, neither automatically nor manually. The changes also blocked the main thread of the application with a synchronous network call that sometimes took very long (depending on the user’s network) and sometimes just hangs and crashed the app.
Since there was no style guide for the developers, the code looked differently everywhere you looked, and no linters enforced any style at all. The copy-and-paste changes that Chad had integrated made for a tedious find-and-replace once they had to fix the bugs. And it was all over the app. Therefore fixing things took another two weeks, and there was no end in sight.

In the end, management canceled the whole feature, because it just didn’t work at all for most of the users. They replaced it with integrating their web analytics engine. A radically simpler approach, but it did work right away.

Similar Posts:

    None Found

Leave a Reply