00:00:05.990
Welcome to the first talk of the day on code reviews. Denys Yahofarov, our speaker today, shared that he recently moved from Ukraine to Germany, leaving behind his beloved Legos. He is patiently waiting for his child to grow old enough to appreciate them, as he enjoys the experience of gifting toys typical for a parent. Let's give a warm round of applause for Denys as he presents.
00:00:20.070
Hello, everyone. My name is Denys, and I have a background in software engineering. I've encountered some amazing and quite unusual things throughout my career, and I'm always eager to explore new possibilities. Currently, I work at Solaris Bank as an engineer, where we are developing a card transaction processing system. When you see the Solaris logo on your payment cards, it signifies that our backend performs all the necessary magic to facilitate those transactions. Now, I want to clarify that my presentation today is somewhat non-technical and purely subjective. It reflects my personal experiences, and while you can utilize my advice, please do so at your own risk.
00:01:22.229
What kind of code do I typically review? I've engaged with open-source code, but my main experience resides in consultancy and product-oriented companies. At Solaris, we have diverse teams each contributing to unique products that, metaphorically speaking, resemble the Legos I left behind in Ukraine. Unlike Legos, however, our code must be organized and interconnected. It's essential in any product company to adhere to deadlines and deliverables; while these usually create pressure, our aim is to build something impressive and also generate profit since we are a bank.
00:02:05.460
So why do I conduct code reviews? I seek to acquire new knowledge and insights. Sometimes, people solicit my feedback on code, while other times, I take the initiative to explore pull requests simply because I'm curious about new developments. This not only allows me to learn but also provides an opportunity for others to benefit from my comments, and ultimately it aids in coaching and mentoring team members. Engaging in discussions about proposals and ideas can happen asynchronously and in close proximity to the code being analyzed, enabling visibility for everyone involved. Moreover, maintaining a healthy codebase is critical, which can mean following conventions, ensuring sufficient test coverage, or even adhering to formatting standards.
00:02:37.520
At Solaris Bank, it is a regulatory requirement that at least one engineer reviews any code before it is merged. This protection means that no one can directly push code to the master branch. If you commit a change to a pull request after receiving feedback, it dismisses previous reviews, necessitating a fresh review. This can be cumbersome, especially given the fast-paced environment where we need to continuously build and move ahead. These circumstances often lead to less time available for writing code than what is needed for reviewing others' work.
00:03:14.070
We employ an open-source model where teams can fork a project they are responsible for, implement changes, and submit pull requests for review. Sometimes, a team may want to advance features that are on our roadmap for the future but need them implemented sooner. Although this quickly increases our workload, it's necessary to allocate time for reviews, which is a critical responsibility.
00:03:51.680
In my experience, I have developed several principles surrounding code reviews that help me manage the process effectively. Firstly, if the code contains value, it should be deployed to production unless it is strictly a proof of concept that will not be merged. The world of software is imperfect, with always room for improvements and follow-ups. Moreover, respect for contributors is paramount, regardless of their experience or background—they deserve recognition for their input. When making comments, I consciously consider how the contributor may perceive my feedback, emphasizing that time is of the essence for everyone involved.
00:04:20.460
One guiding principle is finding something positive in any code I review. Even in code I believe is bad, I make it a point to identify at least one commendable aspect, which may require looking closely to find it. My approach is to evaluate the overall structure and functionality first, ensuring that the code is fit for production and meets acceptance criteria backed by sufficient tests. I believe in a test-driven development approach, where I prioritize examining the tests first to ascertain their effectiveness.
00:05:00.470
In terms of style and syntax, I avoid nitpicking small formatting issues. Instead, I rely on automated tools like RuboCop to maintain code quality. If these tools haven't been integrated into the project, I may note such observations but won't dive into micro-level critiques regarding alignment or formatting. When pull requests are ready for review, I look to merge them as promptly as possible if the code is sound and passes necessary tests. The fact is that code reviews are a regulatory necessity, but there’s no requirement in our procedures for the original contributor to merge their code; under continuous delivery and integration practices, I can sufficiently manage this on behalf of my team.
00:05:52.870
Moreover, I prefer to create a pull request back to the original one if I have feedback on how the code can be improved. This approach allows me to show my proposed changes and keeps the dialogue more engaging. The contributor, in turn, can decide whether or not to address my suggestions, maintaining a collaborative atmosphere where each party respects the other's time constraints. Ideally, my comments are a balance between adding immediate value and recognizing potential technical debt.
00:06:30.920
I focus on the most critical aspects of code changes, avoiding excessive granularity in feedback unless it’s absolutely necessary. I start by discussing broader architectural aspects before drilling down into specific details, as aligning on the macro view often alleviates concerns that might lead to heated discussions about minor issues. Even if some learning opportunities are missed along the way, the number of ongoing developments and changes keeps the overall learning experience robust.
00:07:06.320
Every feedback I provide comes with the understanding that I may not have the full context, meaning there are sometimes valid reasons for something I might critique. I try to emphasize that contributors do their best given their circumstances and constraints, and I make it my responsibility to ask questions to understand things better rather than jumping to conclusions about specific implementations. I believe in maintaining professionalism and providing constructive feedback, as I've once faced my share of unsatisfactory comments on pull requests which didn’t provide any clear guidance on how to improve.
00:07:54.320
I keep my comments clear and straightforward, and I prioritize using simple English, especially as a non-native speaker. I avoid using emojis or ambiguous language unless absolutely necessary, and I look to provide concrete examples when suggesting alternatives. At the end of each review, I summarize my main points, indicating when things are minor and that merging is feasible. This practice streamlines communication and can significantly reduce back-and-forth discussions that often consume precious time.
00:08:22.570
When time becomes a constraint, I prefer to have direct discussions instead of lengthy written feedback, which can lead to miscommunications. Many issues can be resolved quickly in conversation, as verbal discussions often incorporate a larger share of nonverbal cues. After these discussions, I reconvene to summarize our outcomes to keep a written record of what was decided for the future reference of our team members who weren't involved in the conversation.
00:09:16.680
I strive to maintain a positive atmosphere regardless of how I perceive the quality of code. I highlight what I appreciate about a submission, reinforcing good practices, and I also emphasize that I have learned something new from the codebase. This acknowledgment encourages a culture of knowledge sharing and growth within the team. While reviewing code can be taxing, my aim is to ensure the review process is a productive exchange that aids everyone involved.
00:09:55.420
As I conclude, I recognize that micromanaging contributions by rewriting them doesn’t scale—especially in teams with growing numbers. My focus is on fostering a non-judgmental environment where individuals can feel valued and respected for their contributions. If I expect others to support me in similar manners, I must contribute to that effort myself. I encourage everyone in the room to reflect on their own experiences with code reviews, as it can help shape a culture of continuous improvement.
00:10:48.330
With that, I’d like to open the floor for questions. I believe we have around five minutes for discussions if anyone would like to share their thoughts or ask any questions.
00:11:10.910
One participant asked how to handle feedback when it’s not well-received, especially from someone who may own the code. I responded that open communication is key. Engaging with them directly about the feedback tends to be most effective. If they don’t respond to this, I may create a pull request that improves the segment in question. If the contributor isn’t willing to merge it, I look for ways to understand their reasoning better.
00:11:29.450
Another discussion point raised was regarding pair programming compared to code reviews. I confirmed that we do practice pair programming frequently, but we must maintain formal reviews due to regulatory constraints. In our team, even if two people collaborate on code, we still need to have that formal GitHub review to adhere to rules. So, while pair programming can expedite feedback and foster communication, we still align it with our review process.
00:12:05.680
Another participant inquired about managing time effectively to balance between writing code and conducting reviews. I shared that in our process, I usually prioritize reviewing contributions from my immediate team as it helps unblock them faster—this is particularly important if they have a deadline or an upcoming demo. This approach allows me to manage my time efficiently and ensures that tasks move forward without unnecessary delays.
00:12:28.299
As we wrap up, I want to thank everyone for their engagement. Code reviews are a vital part of our development culture. If we share openly and support one another, we can create a constructive environment where everyone thrives.
