DEV Community: Amanu

RFC: Evaluating Value vs Pain for Code Reviews

Amanu — Wed, 08 Nov 2023 13:56:54 +0000

Hi Esteemed members of Dev.to

I am working on creating a questionnaire to evaluate the performance of Code Reviews. As you may have already seen on this year’s State of DevOps Report, code reviews have a huge impact on organization performance. It certainly makes a great argument for pair programming.

In order to surface the miss-givings of code review culture and process of a team, data collection is a necessary step. Hence, I’ve taken the first step to designing a questionnaire that would help provide data to answer the value vs pain/loss of pull requests. I would like to get some comments on my questionnaire design.

Here are the questions I am trying to answer:

How much time is lost in waiting for code reviews?
How much of a developer’s time is spent reviewing code?
Are they accomplishing their target or preventing bugs and design degradation?

Please leave me your comments on what you think about the questions. I would like to know

If the questions were clear and unambiguous
If the options were sufficient and fair
If you think I should add more questions
Or maybe you think there is a better way of accomplishing my goal

The Questions

Here are the list of questions I’ve developed so far.

Are you a default/required reviewer?
- yes/no
During the past month, how much time did you typically dedicate to reviewing code in a given day?
During the past month, how many code reviews did you perform per day?
During the past month, how many change requests took you longer than 15 minutes to review in a given day?
During the past month, how long in average did you have to wait for your code to be reviewed?
When you review code, how much do you agree with the following statements? More often than not, I
- can identify the changes made
- have a clear understanding of the changes made
- have a clear understanding of the context (reason) for the changes made
- have sufficient time to conduct a comprehensive and high-quality review
- feel confident to approve after a review
What is the average size of changes you review?
Do you feel responsible when code you approved introduces issues in the production environment?
How valuable do you perceive code reviews to be (Very Valuable, Somewhat Valuable, No Value)
- maintaining code quality
- knowledge sharing
- preventing functional/logical issues
- preventing performance issues
- preventing security issues
- maintaining system design/architecture
- fostering team collaboration
What do you primarily focus on when reviewing code change? (Select 3)
- maintaining code quality
- knowledge sharing
- preventing functional/logical issues
- preventing performance issues
- preventing security issues
- maintaining system design/architecture
- fostering team collaboration
How often do you give or receive comments regarding: (Very frequently, Here and There, Rarely)
- maintaining code quality and formatting
- functional/logical issues
- performance issues
- security issues
- system design/architecture
When you create a change request, do you trust reviews to identify bugs (functional, logical, performance, or security issues)?
Of the functional, logical, performance, or security issues discovered during QA, how many of them could have been identified during a code review?
What are the frequent reasons for buggy code getting past code review?

I hope to hear from you; Adios

I was wrong! You should definitely push to main

Amanu — Fri, 28 Jul 2023 07:36:23 +0000

“We favor the comfort of conviction over the discomfort of doubt.”
―
Adam M. Grant, Think Again: The Power of Knowing What You Don't Know

You should be pushing directly to main!

I don’t feel comfortable saying that. And if you are anything like me, you have recoiled at that. But now that I have seen the error of my convictions, I am going to start the uncomfortable journey of changing my habits. And I will try to share with you what convinced me to question my love for pull-requests. Here is how it started:

Early last week, I received an email with a link to this blog by Trisha Gee. She listed 5 reasons why she prefers trunk based development. My first reaction was “Hell no! There is no way that this could be efficient.” If that sounds like I responded without thinking deeply about it, it is because I did. It was my intuition to say “Hell no.”While I generally trust my intuition, I know to question it as well. So I took a deeper look at trunk based development.

While I agreed with the benefits of pair programming and how TBD can work, what triggered me about Trisha’s blog was the hostility towards pull-requests. My response to her blog sparked a very interesting internal discussion with my esteemed colleagues here at kreuzwerker. And they told me that they don’t even have a short-lived feature branch. They were just pushing to the trunk (AKA main). (┛ಠ_ಠ)┛彡┻━┻ I couldn’t understand how they couldn’t see the problem with that. I couldn’t fathom how pushing to the single source of truth without any safety check is a practice preached by our industry leaders (such as Dave Farley).

Don’t get me wrong, I have seen the treacherous roads of git-flow. I’ve seen it work for a minute and then stop working, leaving behind a planet sized monolith with no tests and a flock of QA testers required to keep it running. Even the creator of git-flow Vincent Driessen has updated the blog where he introduced git-flow to add a warning. What I was advocating for was TBD with mandatory pull-requests, which I later found out is called Scaled TBD.

I love/loved pull-requests. I use them all the time. Even for myself on my personal projects. I put my neck out to defend them. So, I started writing a blog to set the record straight, to show them the errors of their ways. The great thing about working at kreuzwerker is that I get to have my ideas challenged by folks like Sharib and Katja. And boy was I wrong (well sort of, I can explain). And here I am writing a completely different blog than I had set out to put out.

Why did I love Short-Lived branches and Pull Requests?

Flexibility and Confidence

Anyone who knows me enough knows how much I love refactoring. I’m a good Scout. My pull requests often contain 15 file changes because I either renamed a class, reformatted the code or even upgraded a library causing a cascade of changes. I am usually worried that I might break something.
So, not only do I need to get it reviewed, I would also like to take a serious second look at it. I do a Pull Request review on my own code before I ask anyone to do that for me. I check if the unit tests are all green. I usually don’t even run integration tests on my machine unless I really have to. They are simply too expensive and often unnecessary. I push my changes and let my CI/CD service run the tests for me. I also have a quality check like sonar running as part of the pipeline. All that gives me a lot of confidence that my changes are A-OK to make it to my precious main.

I am also splitting the refactoring commit separate from the actual feature change. That makes for an easy communication of intent.

Clean Commit History

I like my commits to be meaningful. But you know how it goes, there is always something. I often have a lot of meaningless commits. It could be because I was doing a refactor or because I was going back and forth between tasks. My short lived branches give me the security and flexibility to just commit, switch to a different task, complete it and come back. I also use commits as checkpoints. The changes that are not yet committed always tell me what I was doing yesterday. With a PR and short-lived branches, all these meaningless commits are not a problem. You just need to do a git rebase origin/main -i (AKA interactive rebase) when you are ready to create a PR. This way, I get to fix conflicts once and without introducing a merge commit (like using Gihtub’s Squash and Merge). That is a sweet deal.

I sometimes also discover that I have a heapdump file or other unnecessary files checked in by mistake. I’m not afraid to admit that I’ve checked in my maven target directory by mistake. Checking the diff on the PR is usually how I realize my mistake and clean it up before it goes into my precious main.

Quality Control

Other than convenience, my biggest reason to love Pull Requests is that we have a quality gate. You have a dedicated place to make sure you are producing quality code that is well tested. In your Pull Requests, you can gain insight into:

How big or small your tasks are and how well they are broken down
How clear the tasks were
How much effort it took to implement the feature
Quality checks tell you how well the code was covered by tests and if it follows team practices
Is everyone in the team contributing equally

These are a lot of reasons I listed. I did love pull requests. I still do.

So, why I changed my mind

Maybe you have already picked up on a pattern there. And you may be saying ‘duh’. But then again you could be someone like me, waiting to be inspired.
There are a few things to unpack.

Trust

Do you trust your team to follow good practices? Do you write good tests and execute them often to make sure everything is green?

Because if you don’t, then whether there is a pull request or not, it doesn’t really matter. The reviewer can be as negligent as whoever wrote the code. If I can ignore failed tests, so can the reviewer. Unless you are creating a hierarchy of reviewers, there is really no point in not trusting the team. The team functions in a system of trust and responsibility. Pull Requests create a false-sense of security that doesn’t really exist. You can and should trust your developers. Or at least cultivate a process that promotes a culture of trust and collaboration. And that’s exactly what pushing to the trunk does.

If you break the trunk, you break it for everyone. While this may slow down teams that are not yet mature, I believe it will help develop a good culture. For example, if your changes are small enough to be understood by everyone, anyone in the team will be able to help fix your breaking changes. It’s okay to make mistakes; the key is to ensure you have a way to bounce back.

Planning

Do you spend time thoroughly breaking down your tasks? Are these breakdowns meaningful and require reasonable effort? Are these tasks likely to be completed within a day or two?
Because if you don’t, you’ll have tasks that require huge effort to complete. You’ll have a task that spans weeks and sprints. You’ll end up with a huge amount of changes and even pull requests will likely just be formalities. In most cases, reviewing a big pull-request is as good as no review. You might as well have pushed it directly.

Incentivise developers to participate in the planning and task breakdowns sounds like a better strategy. Instead of relying on pull-requests, developers will work to make sure the tasks are small enough to confidently push to the trunk. Developers will have reasons to implement feature-flags and other solutions to make it safer to push to main with confidence. A task that takes a painful week will elicit careful consideration in the planning phase. Isn’t that what we want with agile?

Do you finish work before starting new work? Do you value quality work over speed?

Because if you don’t, you’ll have a dozen works waiting for review. You’ll stack up investments that have not yet yielded, which is what pending pull requests are. Having a pull request process will likely allow developers to move on to a new work before finishing the previous task. It allows developers to off-board the task of ensuring quality to the reviewer. This, I’ve come to see, is not sustainable.

Pairing and Collaboration

This was the biggest factor for me to change my mind. If you have read between the lines, you might have noticed that I like to work solo. I do; I like to go fast and the way to do it is by myself. But that doesn’t necessarily mean going fast as a team. In fact, I’m still hungover from my last role where I realized late that I manufactured a lot that may not survive after I left. I did produce fast, but it ended up slowing everything down in the end because all of the sudden everything needed me to function. Bus factor = 1. As much as my ego liked it, it was disappointing to think that I have left a legacy that I’m not so proud of. It was a stark realization that my work may now be that piece of legacy code everyone is afraid of messing with.

And I did create Pull Requests for all these changes I made. Not always, but I did. But it was the same whether I created a Pull Request or not. Reviewing 100 lines of terraform code wasn’t something my colleagues wanted to do. They also didn’t have the skill. But the pull requests gave me the false sense of confidence.

It definitely would have been slower at first. But I now wonder how much slower it would have been in the end. How much more could we have done if I wasn’t needed to make any changes. How much more could I have done during my notice period if I didn’t have to onboard my colleagues on all that legacy stuff.

Conclusion

I still like pull requests, they are great in many ways. I love working asynchronously. They are a good place for quality checks and to have discussions. They are hard to replace in open source and large projects with large teams. Pull requests are still needed by teams that are yet to mature.

But I’ve come to realize how damaging they can be to the culture and how much they take away from pairing and working together. I’ve failed to realize reviews are not silver bullets. I will definitely miss switching context as easily, but then I shouldn’t have to. It will feel slow and awkward, as any change would.

I am not a fool. I realize how difficult the change could be. But here is an idea. Go without Pull Requests for a month. Force the team to pair on changes, to run tests locally and to push everything straight to main. Maybe that is all you need to start the journey, or maybe you will figure out what is blocking you from doing that.

Because, my friend, for me it was all about fear.

/* P.S: TBD is not easy. It requires maturity of teams, maturity of pipeline and even developer workstations that can run integration tests without that becoming a hurdle. It’s also easy to implement it incorrectly and then blame the practice. I, as much as I would like to, can’t offer you any good tips here. But I hope you’ve found my insights helpful in giving it a go. */

Book recommendation: The WEIRDest People in The World

Amanu — Sat, 01 Jan 2022 21:32:45 +0000

I will start by admitting that it took me a while to finish the book. It started as a very easy read and it got a bit boring in the middle and then it picked up again.

This was a particularly good read for me because it combines a few of my most beloved sciences. I mean other than computer science, which is of course my top Jam.

In this book, Joseph Henrich discusses how the western psychology is actually peculiar and not by any means representative of "human psychology". He argues, because most researches about human psychology in not all were done in western societies, what we now know as psychology is really not human psychology and merely western psychology. And he proves it based on a lot of data. As an Ethiopian man living in Germany, I can testify to the big psychological differences

The main argument behind this is rooted in cultural evaluation. Out evolution as humans have been predominantly cultural since we settled down and maybe a bit before that. Rather than natural selection, we evolve culturally. And that seems to be the reason for out success. This means different cultures bring different psychologies. This means it is of little sense to talk about psychology without acknowledging culture and environment influence human psychology.

While making that above point he discusses how the western psychology came to be. He discusses how the Western Church and it's rules around marriage and family created a wave of changes that brought about a vary weird psychology which triggered other sets of psychological peculiarities. These peculiarities include individualism, impersonal trust, time thrift, guilt over shame, none conforming and etc. Then he goes on to discuss how these peculiarities foster innovation, democracy and economic development.

There are quite a lot of interesting insights in the book. But here are some of the most interesting ones for me.

Protestantism contributed to literacy. How you may ask. One of the basic principles of Protestantism is the need to form a personal connection with God. And the way you do this is by reading scripture for yourself and then understanding it in your own way. This created a wave of literacy. And this literacy wave included women.
Does marrying your cousin sound weird or even appalling to you? It seems like it's very common elsewhere and it has been common for most of human history. I had not realized it was so common. And there is data to support it.
Polygeny is not only interesting for men, it was also interesting for women in ancient societies. Compared to monogamous societies, polygenous societies offer more desirable men. That is because married desirable men (best hunters for example) are not taken off the market. But polygeny also leaves a portion of men without marriage prospects and increases crime.
Monogamy, with men involved heavily in family affairs and raising children, decreases testosterone levels in men and that simple means less crime.
The BIG-5 Personality Traits are not universal. Using a research done in a primitive society, he proves that. In fact there is only two trait in that society. That was mind-blowing for me
War and other disasters, which the west endured for so long, foster in-group cooperation and inter-group competition.

This was a very good read, I highly recommend it. It gave me some insights on possible reasons why western institutions are not working well in Ethiopia and the bigger Africa.They simply don't go well with out inherent psychologies.

If this inspired you to read the book, let me know! I would love to hear your takes on it.

Is it `On Production` or 'In Production`

Amanu — Thu, 18 Jul 2019 13:50:40 +0000

I've got a little bit of Non-native speaker syndrome and I would like to get a second opinion on this.

Which one is correct.

The new version of some-service is on production?

The new version of some-service is in production?

Review my Clojure code

Amanu — Mon, 20 May 2019 11:10:22 +0000

Hi Everyone,

I'm starting to get a hang of Clojure and I started working on a project. I would be a great help if anyone gave me feedback.

Pull request

Library vs Frameworks: My own two cents

Amanu — Fri, 12 Apr 2019 14:27:01 +0000

About a week ago, I was reading a brilliant blog titled “Anyway, this is why I prefer Vue over React”.

He called Vue a library, to which I commented on otherwise. And it sparked an argument that lasted with “let’s agree to disagree”.

Here is what I think

Vue is a framework.

In my opinion, the main distinction between framework and library is who calls who. And callbacks doesn’t count.

Frameworks impose a structure the client must follow in order to make it work. They depend upon the structure you implement to manage the life-cycle of activities. Vue does just that. Let me explain why

When you define a Vue component, you define some methods and objects.

new Vue({
  data: () => {
    return: {}
  },
  computed: {},
  mounted: () => {
  }, 
  watch: {
  }
})

If this was java or any other real object-oriented system, you would have been extending some base class or implementing an interface. Annotating your methods or sometimes just following some naming rules(like Vue). Then the framework decides to call what and when.

When you use a library, you will not be implementing any structure or API, you’ll be calling the API on the library.