hey stop nitpicking in code reviews you
know how many times I would receive
these nitpicks that I absolutely hated
my worst nitpick that I hated the most
the absolute most was that we had a
convention or at least someone had a
convention and I did not agree with the
convention I refused to do stupid
conventions and one of them was to
underscore your variables that are
private in typescript and I said yo dog
it's a typescript library and typescript
disallows you to access private
variables and their response was well
actually you can by doing at at you know
at TS ignore and I said yeah but if
you're doing test ignore you're
fundamentally doing something wrong stop
underscoring something that doesn't
exist anymore that notion that idea
comes from a time when we just had
JavaScript now that we have private and
public modifiers you don't need it
anymore it's stupid to do that don't do
it okay if somebody goes off the rails
and accesses something that's been
marked as private and ignores that your
linter typescript guess what that's them
being stupid it's not on you okay you
can give somebody a gun and they can
shoot themselves in the foot that's
their problem that's their problem okay
it's their problem they pointed at their
foot and then pulled the trigger don't
do that don't shoot yourself in the foot
and so those kind of nitpicks drive me
up a tree right it's like no F your
conventions F that because it makes no
sense we're naming the variable without
that okay one of the best changes I've
made at work recently is to stop
nitpicking uh in code reviews I Dan Liu
I love you I love you Dad that's oh I
love you oh nitpicking isn't about code
that is wrong but sub-optimal it's
pointing out a variable name that could
be used more appropriate like a more
appropriate word a condition that could
be formatted more cleanly or some minor
simplifying of logic nits don't result
in significantly better code nor did
they educate the developer they're just
changes that are technically
improvements if not highly meaning uh if
not highly meaningful ones uh I'm not
really sure about that last one
um I would say that see that's the
problem it's like even okay get this
here's a good example uh we're in some
JavaScript and someone does new promise
uh res reject blah blah blah blah I've
been nitpicked on this it's just like yo
dog
yo dog how about you don't
how about you don't okay in the context
where they're at it doesn't matter or if
somebody goes like this right you have
an array of numbers right and you do a
reduce and you do an ack X act plus X
zero right
if someone's like yo you should rename X
because we don't use single variable
names you're just like stop
stop yo stop
okay ain't nobody caring about that
stop you know what I mean it's just like
that kind of stuff is just so effing
annoying now if your variable name is
loads from data source and it actually
goes and reads out div elements data
deserves a rename deserves a rename
because it's fundamentally wrong totally
agreed but if you believe there is a
better variable name than the one that's
chosen it's likely because for your
brain you understand it better now some
people won't understand it better that's
the thing is like people totally forget
the fact that perspective comes into
every variable name and when somebody
does like you know does it one way and
somebody else does it another way like a
really good example is is in uh you see
this more and more is like if somebody
has some sort of class with something in
it right and you do something like uh
time and it returns you know uh this dot
current time I don't know look at that
nice underscore I did that just to piss
somebody off versus get time right
these are two these are the same thing
and people argue over this now because
some people like the former approach
better where some people like the latter
approach better and this is like totally
uh your experience gets you into this
place
you know what I mean your experience
gets you into this place and if you are
super used to something then You're
Gonna Want A Time versus a get time and
that's just that and for me I prefer
time I don't see why I like get time
Some people prefer get time with the
capital G because they're c-sharp losers
okay I want a code base to be perfect
and in my mind these nits were an
important part of ensuring that happened
I used to conduct extremely thorough
code reviews filled the comments about
architecture and bugs but also so so
many knits the knits often overwhelmed
the other comments for every major
concerned I'd raised there could be a
dozen nits this is another great Point
great great great great point right
there this is such a good point when you
overwhelm people with a bunch of nits
how hard is it to make the proper things
fixed you know what I mean if you see
something that's fundamentally wrong
address it
right
that is way way better than being like
you know what your underscore in that
private they were missing underscore in
that private variable you're like
you just missed the whole thing you know
what I mean this is practice of mine uh
did not make me the most popular
co-worker far from it I didn't realize
it at the time but my perfectionism was
toxic at best it annoyed people at worst
it upset them and those feelings lasted
far past the code review itself exactly
there's people on Netflix I don't want
to work with because they're like this
right like I'll avoid working with them
which is bad bad on my behalf but
there's nothing you can do to get away
from it now this is what they're known
for is being Ultra nitpicky it's just
like you gross I don't want to work with
you because here's the deal anytime I go
into code I think it looks like no
matter who wrote it when they wrote it
including if that person was me if it is
more than a week old it is pure ass code
every single time every single time
every time
a couple years ago a co-worker proposed
that we uh see what happens if we
stopped nitpicking altogether initially
I pushed back I knitted The Proposal why
would I want to allow bad code to get
into code bases but in the end we agreed
to a one month experiment what a great
thing right here I love this also
another great sign of a healthy team are
people who are willing to go against
what they think and just try something
out this is awesome to my to my surprise
at the end of the month things were
cleaner uh better than before were
clearly better than before so much so
that we barely even discussed whether to
continue the policy it was just sort of
became a permanent because it was
obviously an improvement uh there were
two big benefits we experienced this is
such a well done article right now Dan
Liu you did incredible this is
incredible this is this is awesome first
we saw a vast improved signal to noise
ratio absolutely absolutely
um it's just like if you have a CI
that's kind of flaky you know a flaky CI
and you have that one test that fails
kind of regularly who here's merged on a
broken test because of that one flaky
test that's kind of flaky who here's
done it
who here's done it this guy's done it
right here this guy and guess what one
of those times it wasn't flaky
it was broken
you know what I mean but that's the
problem about signal to noise if you
have something that's noisy people will
Overlook things
you know what I mean imagine a code
review that gets results in five nits
and one critical issue to address in the
hullabaloo of fixing those Knits
the critical comic can seem less
important or even get overlooked not
only that but if they ask you to
refactor a couple things pull this out
as a function or go do that the critical
bug sometimes disappears from the code
review and will be missed because now
you have a restructured version that you
can't quite see what is Beast doing one
one one one one one one yeah me and bsco
were a lot alike not a Hubble a
hullabaloo yeah a hollow balloon uh
without nits okay what is going on with
my voice I can feel something in there
sometimes the critical bug is also fixed
by refactoring absolutely and sometimes
you introduce a new critical bug by
refactoring we call that refactoring
um without nits that one critical
comment is the only issue that gets
brought up all the attention is paid to
it and right rightfully so what is
important gets a signal boost what's
important what's unimportant gets
ignored yes second it improved
everyone's relationship with code
reviews and those who conducted them
I've seen plenty of people talk online
about how you shouldn't take code
reviews personally that's uh that it's
the code being critiqued not the person
blah blah blah blah blah it's a bunch of
I've been going through code
reviews for a decade and it still stinks
when someone points out my mistakes or
pushes back on my code designs imagine
used I I actually don't really feel this
too much
I just don't care like that to provide a
good reason for the most part I think
that most people when they push back
it's just a bunch of nits and then I
don't care imagine you start a pull
request and the first reviewer points
out a dozen nits your natural response
will be uh will not be zen-like
acceptance it will be to get upset uh
all you want to do is merge this code
but some jerk is making you jump through
a bunch of dumb Hoops to do so first now
or suppose now that you fix a nitpicks
and send it back to be reviewed only to
get blocked again because the reviewer
noticed some more nitpicks yep yep
developers like to imagine they are are
composed of nothing but cold hard logic
but actually we are humans with
unavoidable feelings and emotion uh what
I what was what was I really doing then
or when I was thoroughly nitpicking code
the main effect wasn't improving the
code base it was making people upset
with their uh code and angry with me I'm
so motivated by this uh
I think uh oh my goodness I have a tweet
coming I have a tweet coming
I can't do it yet it's not there yet
it's not I gotta get sufficiently more
upset hold on I gotta get sufficiently
more upset at what I'm about to say and
then we'll come back to it we'll come
back to it I can feel it coming though
Something's Gonna Come Out of me I can
feel it
um
uh they were less receptive to my
feedback as a result and it soured my
relationship with my code workers a
couple years after stopping knits people
are far more receptive to feedback and
code reviews and I think much less of it
and and think I'm much less of a Jerk
It's a night and day difference from
before ultimately our code base has not
suffered due to lack of nitpicking if
anything it is better than before
because we as a team work together
better and focus on the important parts
of the code design in our reviews
let's go but what if you still care
about what uh what you nitpicked before
my suggestion is to automate what you
think you'd otherwise nitpick add lint
checks and code style enforcers to your
CI absolutely this is like the simplest
thing is just agree on a set of prettier
rules
you know you're gonna have to push back
on some things you and at the end of the
day somebody you have to designate a
time box for argument
and a way to resolve all conflicts and
just do it you will argue over any rule
no more than five minutes and if there
is a pure tie you flip a coin right and
that's just that and you move on and and
just who cares right I'm a four space
indenter some people are two space
indenters I think two space indenting is
dramatically worse for a code base
because it allows you to indent more
without feeling the pain of indenting I
also only choose 80 line characters
right here okay I when I well this is
templating templating you have no choice
on and HTML you have no choice on but in
JavaScript I enforce 80 lines why
because it makes me think about
refactoring much much more it's a
personal opinion now some people don't
like that some people like tabs I'm a
I'm a Force Base indenter that's just
the way it is uh it will always be that
way 40 40 and 80. hands down the 480 is
the best uh with tools you can check
problems in advance plus when automation
tells you you're incorrect it's
impersonal and a result of somehow less
frustrating yeah it's just because it is
what it is but seriously if uh you're
like let's see if you are like how I
used to be someone who wanted every line
of code to be perfect just try letting
it go a bit check your a low Gaff before
you comment
what the hell's a low Gaff it's another
damn can you just tell me what the hell
it is what the hell is a low gaff
level of give a ah yes
I think everybody needs to check that
level and adjust it a little bit you
know what I mean
because here's the deal is that the only
things that are going to screw you are
bad Logic the only things that are going
to screw you are really obnoxiously long
functions that need to be refactored
that's it
and you know and lastly the thing that's
going to screw you is uh too much unit
testing or not enough unit testing so I
built like recently here's a good
example I recently built a bit of a
Conway
a Conway Game of Life thing and to make
sure my Conway Game of Life stuff works
in this little JavaScript thing I was
building I just put a simple test I
wanted to prove to me that I could do
Conway correctly and this didn't cover
all the cases uh but it covered enough
of the cases for me to be happy right
and so for me that's a sufficient I like
I don't need a ton of tests to tell me
when things are wrong I could have added
one more to have the four or more
population will kill out uh you know a
pixel I didn't good enough don't care
you know what I mean you know what I
mean yeah I feel like people sometimes
way over test things there's like 1800
tests to test every possible case of
every little thing I'm like
you know that's not the hard part
you know the hard part
isn't there right you've done something
or people test like the dumbest
functions have you ever seen that where
someone will test like like just like a
literal for loop it's like uh function
Foo and it's just like uh four let I
equals zero is ISB less than ten oh my
goodness 10 I plus plus uh return if uh
you know uh if I equals five oh return
drill
return false I mean this is terrible
code but you get the idea uh it's just
like
stop do we need a 10 000 test to run I
I don't care about code coverage I care
about code that's hard okay if I can't
get this right first try
I tested pretty much my general rule of
thumb and so I don't know I have a bunch
of things where I think that people go
off the rails and what they require and
then it just ends up being such a huge
pain in the ass at the end of the day
it's just one gigantic ass pain and it
really never made your code base better
we have we have we have tests at Netflix
like 10 000 of them and 8 000 of them
run every single time and they provide
nothing they've never provided anything
they've never provide anything they'll
just keep on being tested because that's
just what it is and I feel bad about
that I don't want that there you go
that's that that's my personal feeling
get out of here don't nitpick so much
okay stop nitpicking the name is don't
nitpick my code or I'll ignore you I'll
block you on Facebook okay