Transcript
This transcript was autogenerated. To make changes, submit a PR.
Hi everyone. Welcome to my talk about code review.
Alternatively, best practices tips and how to not drive
your colleagues into depression with your comments. And without further ado,
let's get started. I'm pretty sure that most of
you know what exactly is code review. But just to reiterate
on what exactly we're talking about here, code review
is just review of codes written by the colleagues.
It's as simple as that. Other colleagues write changes
to the code, they publish them, they suggest these changes,
and then we comment on these changes. Or we
just approve without comments, depending on whether we like the code or
not. Do we even need code review actually, or why do
we need it? Because, well, developers could be
doing something else, right? Instead of arguing comments, checking other people's
code, they could be already spending time on writing new features.
So maybe we don't need it at all. Well,
first of all, more eyes on code means more chances
to catch potential bugs or things like privacy and security issues,
right? It's like additional layer of testing, if you may. If we don't catch
those on time and we actually release the code,
it may result in potential production downtime. It may result
in regulator problems or even ruined customer
trust. So from this point of view, we are interested in catching bugs
as early as possible, which means maybe during review stage.
Alternatively, code review can also mean sharing best
practices within the project or even the company.
One programmer can comment on something suggests
another approach which is used, and this ensures that
another developer or a group of developers know the code styles.
And this can even actually help developers progress
professionally if they get some comments about
an approach, for example, which they hadn't known before.
Finally, code review can stop the
unintended workflows. For example,
usually one developer should not be able to push to production
and verify changes, and code review may act as a security
layer if someone tries to do something bad,
intentionally or unintentionally. Hopefully,
code review may just add a little bit more confidence
that this will be caught early.
So now let's talk a little bit about best practices, which I
think are appropriate in code review. First of all,
regarding a detailed review, if you want to really focus and you
want to make sure that you know and you go thoroughly through authors
changes, it may be helpful to check out the authors branch
locally on your machine and then examine each individual file.
I'm saying this because locally you will usually have your favorite code editor,
which can help you identify syntax errors, and also
you can navigate quickly between files to see the connections.
And all of that may really help you maintain the
context during code review. Also, sometimes, for example,
when I need to really attentively review the code, I book a slot in my
calendar. I put on headphones. Basically, I tried
to ensure that there are no distractions so that I could really
familiarize myself with the context and stay focused.
As opposed to that, sometimes it makes sense to review just syntax
and obvious syntax bias without getting into
the whole business logic. I would say this is appropriate
if you get multiple merge requests per day,
and then you may just not have the time to actually go
through each of the merge requests individually, checking them out
as I discussed earlier. Just I would say
that make sure that the author knows that you are not
reviewing the deep logic, the business logic, but you are mostly reviewing
all the syntax. Now one of my favorite suggestions
is really, really adapt your comment style.
For example, some comment like this may
be appropriate for a more junior person, or the person who
has been working in your project recently, or just
a sensitive person overall, it's a very polite suggestion that hey, can yuri
please remove this serializable term?
And then I explain the reason why was it needed
and then why it is not needed. So you try to be very polite,
you give all the context and just basically make sure that person
is on board with you and doesn't feel anything any
bad things. However, if someone
is more senior experienced or you just know, they will understand exactly what
you are saying. You can just write something that's not needed and it will
be a pointer for them that this is not needed and they will remove it.
And maybe they already have the context, or they will ask you for the context
if they need to. But sometimes this
is enough. So this is up to you to understand which type of person
or which type of style best fits in the given situation.
Beware of emotional damage. Really,
if you put too many comments, I understand we want to comment
on everything. Sometimes there are so many things which can be done differently,
but it still can be overwhelming even for very experienced developers.
Like speaking of my experience, I have more than ten years
and still when I receive a review with
like, I don't know, ten comments or more, my first reaction
is to, I don't know, to try to protect myself, to try
to respond back and so on.
So I would say try keeping only the most significant ones
and maybe move the rest offline. If you
really think that you should write more that your code
review doesn't have enough space for all of your comments,
because the goal is eventually to encourage positive
approach and to be productive and
the developer may risk in not being productive if they
receive too many comments. So yeah, try keeping just the most significant
ones. And if you still need to put more, just move the rest of line.
When you like something, please write about it
as well. We don't only need to comment on whatever needs to be changed.
Well, mostly, yes, that's what we do. But also, it's so nice to feel a
little bit of gratitude. Right? Like you can write very tiny comments like
I like this or thank you, great job. And as
you see, I get hearts for these comments. So it
may make a developer smile, it may bring up their mood.
And happy developer means, after all, more productive developer,
right? And also with a happy developer, it's more pleasant
to work and we may enjoy that more. So yeah, please try
to not always be negative. Try to also emphasize on the good things which you
see. Also, I would say, don't argue back and forth
in comments. Again, it's kind of the same
as when you leave too many comments. People tend to get protective,
and especially because it's a public space. Culture view
is a public space which can be usually viewed by many developers, they might get
the feeling of being publicly attacked. Yes, ideally, if we
were all logical machines, we just only consume the logic of
your comment. We would never actually, I don't know, emotionally react.
But the reality is that we are not. We are all people, we all
have our own issues. You never know which issues are there.
So maybe you could help preventing any triggers by
just following these tips. Like for example,
it doesn't make sense to argue back and forth. If he wrote a comment
and then developer replies and doesn't agree, and then you write back
and then developer writes back, it eventually becomes a battle of egos,
like whose comment is going to be the last? Who is going to win?
I would say if that's not the goal, if the goal is to actually resolve
the issue, just move the conversation offline.
If you see that the developer doesn't agree. Okay,
what is your goal here? Do you still want to push for that?
If yes, sure. Maybe culture view is not the best
place anymore to push for it. Maybe you just
moved to slack channel or moved to oral conversation
or jaspeak any other way of talking.
Just one thing to add here. I would say that if you do this and
eventually they should get resolved offline in the Koji
view, you could make a comment that something like discussed offline or resolved
offline so that if other developers come
back there and they see that you had kind of disagreement
and they don't know what was the result. At least they could
see that hey, it was discussed applying, so if they really are interested
in knowing whatever happens in the end they can actually find you
and ask you what was the outcome. And that is
all for me. Thank you very much for attending this lightning
talk. I really appreciate this. I wish you all
very happy coding, very nice review comments
and yeah, just wishing you to
be very relaxed and productive during your work days.