Detailed Notes
Code reviews are starting to become more common in enterprise development. Dev Ops and related automation have helped shine the light on improving quality and reliability in our processes. This presentation looks at how we can set our teams up for success in code reviews through better and consistent processes. This episode goes over the questions and answers session from the presentation.
Code Reviews In Detail There is a time and place for code reviews. Likewise, there are ways to approach these complex tasks that can make the most of both our time and resources. Nevertheless, this process does not come without cost. Therefore, we need to be intentional in our approach and requirements to craft better code and provide a path for building better developers.
The Mentor-Mastermind Group This series comes from our mentoring/mastermind classes. These classes are virtual meetings that focus on how to improve our technical skills and build our businesses. The goals of each member vary. However, this diversity makes for great discussions and a ton of educational value every time we meet. We hope you enjoy viewing this series as much as we enjoy creating it. As always, this may not be all new to you, but we hope it helps you be a better developer. Drop us a line to find out when the next one is so you can join our group.
Transcript Text
[Music] and now i'd like to open up questions comments hey there rob i wanted to ask uh two questions that may seem uh diametrically opposed um the first is you know you've just got a simple code snippets code smail uh rule just in your head there's an exception that doesn't have a log statement i'm gonna instantly raise a flag and that's simple example but you know there are different uh kind of hard rules that you may go by as you review code um when to when the question is when do we uphold those types of rules versus uh the second question um when do we promote uh some hacky type stuff if i were to give an example of that you know we may prime a dependency inside of a static initializer so that it is ready when the class is being called in in in run time so that makes sense uh that dependency prime may always throw an exception and maybe there's a thorough commentary on why it's there but that said you know it's a hack all the same or you may have some other hacks that the developer may have some other hack that they use when are we going hard and fast you know there's there's no more there's there's no situation in which a lot an exception doesn't have a log statement versus when are we saying hey do something outside the norm here and that's okay that's um that's an excellent question and that is a part of what the code review conversation piece of it is uh and where even if you have automated tools there still needs to be some human uh potential intervention or interaction within it um because within those things like within those coding tools and some of the the automated rules you can put together are things like um if we've got a code review tool that's a developer code review tool like fisheye and crucible or an example when you combine that i guess it's crucible is really the the code review tool you can set it up and say hey is any commit is going to be held um and you can't do a you can't complete a pull request until there are x number of code reviews that are done on the code and i've worked in environments that have exactly that and those are the cases where you may look at it and say okay we've got to do a code review but the person can a person can pass a code review and say yes it passes even though the code maybe is is not up to snuff and so we we want to be able to wow for that but also i think there's a lot of time well there's there's two cases one sometimes it's a hack and we know it and we're going to accept it anyways and it's the it's the 80 20 rule and that's where we as part of that process is just note that this is a hack and we put a little to do in there or or put some sort of a a technical debt uh task in the backlog to say hey we did this it's what we need right now but at some point we want to revisit it and there may be somewhere that's just part of the code review is that we come back and we put a comment on that and say yes this is a hack but it makes sense and it's not worth for us to try to pick up make up a word de-hackify it because the because of the cost or because of scalability or performance or the rare cases where this actually matters um so it becomes a this is definitely where there's a there has to be human intervention there is there is human decision that goes into it and i think it's part of your that's part of your environment is how much do we accept hacks and workarounds and how much do we want to make sure that we we either don't accept them or that we highlight those as things that we do need to come back and clean up and then it goes to you know the old the age-old thing of when do we actually go back and fix it you know are we an organization that says we'll fix it later and then never does or are we an organization that says we can fix it later and there's a certain uh pressure point where we say we've got a lot of stuff we need to fix so we're going to do a fix release or a technical debt release or something along those lines does that answer your your question yes sir yes sir so it's just kind of finding that balance with the human port of it all i like that yes and it's something that's it may be something that you review on a regular basis you know maybe when you first turn on code reviews you have a a lower barrier and then as you get better you raise that barrier it's just and particularly this is this is really good in like the agile sprint world is that maybe early on you have something that's got very uh like for example everything's a warning but then as you go further into your sprints at some point you start turning stuff on and say oh if this occurs now it's an error the eclipse itself has got really good settings built in for it where you can you can change that with a lot of compiler settings and things like that where you can say oh this is like deprecated it's a great example i can in a lot of environments say if something's deprecated then i just want to be warned about it but there's other places you know there's also an option where i can say if it's a deprecation it's an error i want to i need to fix that as opposed to just say oh yeah that's something i want to look at later so it's a it can be a sliding scale essentially or at least a maybe not but a changing scale over time excellent question good deal and i just wanted to oh go ahead um i just wanted to reiterate that um uh cross training i mean too many people shy away from a code review for how they um may uh you know be i don't want to say it's not exposure but you know they may not be as confident in their skill set or whatever but that's how you get better getting other developers to so just different ways and learning from others that's that i will piggyback on that is that um sometimes one of the things i did not bring up in the presentation but it's a good point is that sometimes code reviews are limited to people in the same uh silo so for example if you've got a team where you've got some let's say you've got some front-end people some middle tier people and some database developers sometimes it's that the code review has it's just you know database developers do database code reviews middle tier do middle tier reviews front end do front-end reviews i think part of the value of cross-training is to have the other types of developers be part of the code review it may be initially it probably will slow things down a little bit because they're having to understand maybe some differences in that other uh that other tier or that other functional area area but over time it's going to help them at least they've been exposed to those other areas they have some knowledge about it and they're even going to be able to pick up some of the things to look for in those other coding areas so it is a way to get people out of their coding comfort zone a little bit and cross train them even beyond just other areas that you know like other code within their team or their silo and also you know so they're actually now seeing the whole development group and how they are developing and getting exposed to that so yeah that's it's great to point that out other questions and comments going once going twice i don't hear any so uh that wraps this up a little bit so not a little bit but completely so as always uh i appreciate the time and uh sitting down and going through this uh not only for us you know from the developer point of view but also uh for us the greater community of developers is that when we get together when we spend some time making ourselves better it's it helps not only us but the next guy or gal that comes behind us and has to look at our code or look at the code of other people that we've influenced so you know we it's like a you know pebble in a in a pond kind of thing is that as we get better our codes a little better other people are exposed to our code they get a little bit better and it just builds and builds and builds so this is something that definitely helps all of us it's not just a selfish pursuit but as always if you have any other questions if something comes up if if you want to know more about this if you want other links or anything like that or just in general learn more about us or what we can do or what you can do you can shoot email at info developernewer.com you go out to the developer.com site we have a contact us form we have our rbc posts is where we do is our twitter sometimes to do that i will we had a developer number one we'll probably surface a we'll probably go back and turn back on uh developer door just because of the uh the topics rbc post is much more overall software development we may get a little more deeper we may split up and add another twitter at some point so just sort of keep an eye on that that'll probably be out on the developer nerd site or in some sort of a newsletter if we do that we have a youtube channel i just go out to youtube and search for developer newer and you will find that there's not too much on that uh also i've got a link here if you want to check that out uh vimeo you can go to vimeo.com developmentor you can see some of the videos youtubers order form those tend to be typically the goal is 15 to 20 minutes and
Transcript Segments
[Music]
and now
i'd like to open up questions comments
hey there rob i wanted to ask uh two
questions that may seem
uh diametrically opposed
um
the first is
you know you've just got a simple code
snippets code smail uh rule just in your
head
there's an exception
that doesn't have a log statement i'm
gonna instantly raise a flag and that's
simple example but you know there are
different uh kind of hard rules that you
may go by as you review code um
when to when the question is when do we
uphold those types of rules
versus uh the second question
um when do we promote uh some hacky type
stuff if i were to
give an example of that you know we may
prime a dependency inside of a static
initializer so that it is ready when the
class is being called in
in in run time so that makes sense uh
that dependency prime may always throw
an exception
and maybe there's a thorough commentary
on why it's there but that said
you know it's a hack all the same or you
may have some other hacks that the
developer may have some other hack that
they use
when are we going hard and fast you know
there's there's no more there's there's
no situation in which a lot an exception
doesn't have a log statement versus when
are we saying hey do something outside
the norm here and that's okay
that's
um that's an excellent question and that
is
a part of what
the code review conversation piece of it
is
uh and where even if you have automated
tools there still needs to be some human
uh potential intervention or interaction
within it
um because within those things like
within those coding tools and some of
the the automated rules you can put
together are things like
um
if we've got a code review tool
that's a developer code review tool like
fisheye and crucible or an example
when you combine that i guess it's
crucible is really the the code review
tool
you can set it up and say hey is any
commit
is going to be held um and you can't do
a you can't complete a pull request
until there are x number of code reviews
that are done on the code and i've
worked in environments that have exactly
that
and
those are the cases where you may look
at it and say okay we've got to do a
code review
but
the person can a person can pass a code
review and say yes it passes
even though the code maybe is is not up
to snuff
and so we we want to be able to wow for
that
but also i think there's a lot of time
well there's there's two cases one
sometimes it's a hack and we know it and
we're going to accept it anyways and
it's the it's the 80 20 rule and that's
where we
as part of that process
is just
note that this is a hack and we put a
little to do in there or or
put some sort of a
a technical debt uh task in the backlog
to say hey we did this
it's what we need right now but at some
point we want to revisit it
and there may be somewhere that's just
part of the code review is that we come
back and we put a comment on that and
say yes this is a hack
but
it makes sense and it's not worth for us
to try to
pick up make up a word de-hackify it
because the
because of the cost or because of
scalability or performance or
the rare cases where this actually
matters
um
so it becomes a this is definitely where
there's a
there has to be human intervention there
is there is human decision that goes
into it
and
i think it's part of your that's part of
your environment is how much do we
accept
hacks and workarounds
and how much do we want to
make sure that we we either don't accept
them or that we
highlight those as things that we do
need to come back and clean up and then
it goes to you know the old
the age-old thing of when do we actually
go back and fix it you know are we an
organization that says we'll fix it
later and then never does or are we an
organization that says we can fix it
later and there's a certain uh
pressure point where we say we've got a
lot of stuff we need to fix so we're
going to do a fix release or a technical
debt release or something along those
lines does that answer your your
question
yes sir yes sir so it's just kind of
finding that balance with the human port
of it all i like that
yes and it's something that's it may be
something that you review on a regular
basis you know maybe when you first
turn on code reviews
you have a
a lower barrier
and then as you get better
you raise that barrier it's just and
particularly this is this is really good
in like the agile sprint world is that
maybe early on you have something that's
got very
uh like for example everything's a
warning
but then as you go further into your
sprints at some point you start turning
stuff on and say oh if this occurs now
it's an error
the eclipse itself has got
really good settings built in for it
where you can you can change that with a
lot of compiler settings and things like
that where you can say oh this is like
deprecated it's a great example i can in
a lot of environments say if something's
deprecated
then i just want to be warned about it
but there's other places you know
there's also an option where i can say
if it's a deprecation it's an error i
want to i need to fix that as opposed to
just say oh yeah that's something i want
to look at later so it's a it can be a
sliding scale essentially or at least a
maybe not but
a changing scale
over time
excellent question good deal
and i just wanted to oh go ahead um
i just wanted to reiterate that um
uh
cross training i mean too many people
shy away from a code review for how they
um may uh
you know be i don't want to say it's not
exposure but you know they may not be as
confident in their skill set or whatever
but that's how you get better
getting other developers to so just
different ways
and learning from others
that's that
i will piggyback on that is that um
sometimes one of the things i did not
bring up in the presentation but it's a
good point is that sometimes code
reviews are limited to people in the
same uh silo so for example if you've
got a team where you've got some let's
say you've got some front-end people
some middle tier people and some
database developers
sometimes it's that the code review has
it's just you know database developers
do database code reviews middle tier do
middle tier reviews front end do
front-end reviews
i think part of the value of
cross-training is to have the other
types of developers
be part of the code review
it may be initially it probably will
slow things down a little bit because
they're having to understand
maybe some differences in that other
uh that other tier or that other
functional area area but over time
it's going to help them at least they've
been exposed to those other areas they
have some knowledge about it and they're
even going to be able to pick up some of
the things to look for in those other
coding areas so it is a way to get
people
out of their coding comfort zone a
little bit and cross train them even
beyond
just
other areas that you know like other
code within their team or their silo and
also you know so they're actually now
seeing the whole
development group and how they are
developing and getting exposed to that
so yeah that's
it's great to point that out
other questions and comments
going once going twice
i don't hear any so
uh that wraps this up a little bit so
not a little bit but completely so as
always uh i appreciate the time and uh
sitting down and
going through this uh not only
for us you know from the developer point
of view but also
uh for us the greater community of
developers is that when we
get together when we spend some time
making ourselves better it's it helps
not only us
but the next guy or gal that comes
behind us
and has to look at our code or look at
the code of other people that we've
influenced so you know we
it's like a you know pebble in a in a
pond kind of thing is that as we get
better
our codes a little better other people
are exposed to our code they get a
little bit better and it just builds and
builds and builds so this is something
that
definitely helps all of us it's not just
a
selfish pursuit
but as always if you have any other
questions if something comes up if if
you want to know more about this if you
want other links or anything like that
or just in general learn more about us
or what we can do or what you can do
you can shoot email at info
developernewer.com you go out to the
developer.com site we have a contact us
form
we have our rbc posts is where we do is
our twitter sometimes to do that i will
we had a developer number one we'll
probably surface a we'll probably go
back and turn back on uh developer door
just because of the
uh the topics rbc post is much more
overall software development we may get
a little more deeper we may split up and
add another twitter at some point so
just sort of keep an eye on that that'll
probably be out on the
developer nerd site or in some sort of a
newsletter
if we do that we have a youtube channel
i just go out to youtube and search for
developer newer and you will find that
there's not too much on that
uh also i've got a link here if you want
to check that out uh vimeo you can go to
vimeo.com developmentor you can see some
of the videos youtubers order form those
tend to be typically the goal is 15 to
20 minutes and