►
From YouTube: Code Review Weekly Workshop - Aug 19, 2022
Description
In this session we discuss some best practices pertaining to code review. We also mob-pair on a code review.
A
A
A
A
A
It's
pretty
good
to
have
somewhere
in
the
test,
something
that
asserts
either
the
default
state
that
would
have
been
in
just
a
totally
separate
test,
or
it's
even
totally
cool
to
just
put
it
before
the
action
in
the
very
test
itself
and
one
of
the
things
when,
when
we
did
this,
for
instance,
when
we
click
on
this,
we
would
expected
visually.
The
visibility
should
should
change.
A
That
actually
never
changed
because
is
visible,
actually
doesn't
quite
behave.
The
way
that
we
expected
it
to
so
this
test
wasn't
actually
doing
everything
we
thought
it
was
so
that
happens
a
lot
whenever
you're
doing
an
action
that
could
change
state
to
get
really
good
test
coverage,
asserting
the
default
state,
either
in
a
separate
test
or
in
the
same
test.
That's
doing
the
action
having
a
before
and
after
expectations
can
be
really
helpful.
A
And
the
one
other
point
about
that,
mr
was
not
sure
if
you
all
are
aware
that
we
have
a
whole
process
for
finishing
merge
requests,
and
this
is
usually
done
by
mr
coaches,
which
work
closely
with
the
community
to
help
triage
their
mrs
and
help
community
contributions
get
pushed
forward,
but
really,
like
any
engineer,
can
get
ping
to
finish
a
community
contribution
and
probably
the
biggest
task
here
that
might
be
unknown.
Besides,
just
hey
we're
just
going
to
finish.
A
This
thing
is
to
try
to
preserve
some
sort
of
original
commit
that
gives
credit
to
the
author.
I
don't
think
it
can
actually
be
a
git
trailer.
I
don't
think
our
change
log
process
supports
author
as
an
actual
git
trailer.
I
think
it
actually
has
to
be
the
author
of
a
commit,
so
I
need
to
change
this
wording
because
I
don't
think
that's
correct
and
I
hope
someone
can
correct
me
if
I'm
wrong.
A
A
B
I
was
swimming
in
the
deeps
for
a
while,
so
I
had
to
I
felt,
like
I
had
to
read
into
a
lot
of
around
it
in
context
to
figure
out
what
was
actually
going
on
and
I
felt
like
it
took
me
two-ish
days
to
actually
start
actually
reviewing
the
code
and
not
just
trying
to
catch
up
to
where
this
is
I'm
kind
of
wondering
what
it
might
might.
It
have
been
better
to
hand
it
off
to
a
domain
expert.
C
Yeah
that
one
is
something
I
struggle
with,
sometimes
when
the
mrs
have
a
lot
of
like
already
discussion
on
the
mr,
or
also
a
very
long,
I'm
looking
at
them
or
now.
Maybe
like
a
very
long
description,
I
would
say
I
am
encouraged
by
they
have
instructions
on
how
to
set
it
up
locally.
So
I
feel
like
that
would
make
me
maybe
look
at
it
more
than
if
there's
no
instructions
on
how
to
validate
it.
A
A
Maybe
the
thing
is
hard
to
understand
in
general,
so
I
I
I
appreciate
from
time
to
time,
I'll
I'll
put
mrs
and
I'll
get
people
reviewing
it
and
their
perspective
is
just
so
valuable
because
I
realize
I
realize
that
I'm
taking
a
lot
for
granted
of
how
familiar
I
am
with
this
code
base
and
I
really
appreciate
all
the
questions
you're
asking.
So
that's
I
would
say
yeah
at
some
point,
maybe
maybe
it
is
warranted
like
hey.
I
don't
think
I
can
do.
A
Let's
try
to
make
sure
either
the
maintainer
is
a
domain
expert
or
another
domain
expert
looks
at
this,
but
just
doing
a
flurry
of
all
of
your
questions.
That
would
be
great.
I
I
feel
like
so
you
don't
have
to
feel
like
you
have
to
learn
it
all
yourself
from
scratch,
but
just
saying
I'm
unfamiliar
and
I
have
a
bunch
of
questions
about
it
and
sometimes
I'll
even
just
leave
non-blocking
questions
and
that
can
create
follow-up
issues
because
things
yeah
are
legitimately
confusing.
C
C
D
Joined
late,
sorry
about
that,
I
I
joined
because
I've
just
become
a
reviewer
and
yeah.
It's
super
intimidating,
like
it's
sort
of
like
I
feel
like
I'm
just
sort
of
still
getting
up
to
speed
with
what
my
team
looks
after
and
even
there
there's
a
bunch
of
questions
I
have
so
yeah
it's
it's
kind
of
a
bit
scary.
I
mean
the
I
I'm
I'm
guessing,
because
I've
just
become
a
reviewer.
D
All
of
the
things
I've
been
asked
to
review
are
very
simple
changes
that
they're
just
very
easy
to
approve
so
far
but
yeah,
I
am
kind
of
dreading
sort
of
you
know
getting
pinned
on
something
where
I'm
just
like.
What,
because
a
lot
of
the
merge
requests,
my
own
team,
I'm
still,
you
know,
sort
of
going.
Okay
what's
happening
here,.
A
D
Yeah
when,
when
you
said
that
earlier,
I
sort
of
felt
a
bit
of
relief,
I
was
like
okay,
good
yeah,
I
mean
the
other
thing
is,
I
think,
like
as
a
developer,
I
don't
think
code
reviews
been
a
strong
part
of
my
you
know.
Like
yeah,
I
don't
think
it's
one
of
my
stronger
skills,
and
so
I'm
also
keen
to
improve
stuff.
C
D
C
C
I
check
the
branch
out
get
gdk
up
and
running
and
I
try
if
it's
something
that
can
be
like.
That's
like
a
ui
or
like
an
api
that
I
can
run
also,
especially
with
like
the
graphql
stuff.
I
don't
know
graphql
super
well
so,
like
manually
testing
that,
like
I
come
from,
I
used
to
work
as
a
performance
tester
so
like
I'm
always
like.
I
want
to
break
it.
Yeah.
D
C
C
E
A
A
You
know
you
can
get
locally,
but
or
if
the
pipeline's
passed
and
you
know
cross
your
fingers
and
everything
of
this
thing
deployed
correctly,
there's
usually
a
button
that
says
review
to
view
the
app
and
then
our
login
credentials
are
in
our
our
engineering,
1password
organization,
and
so
this
this
this
could
be
an
easy
way
to
test
some
changes.
Some
changes
need
like
configuration
stuff
before
you've
started
the
jdk,
and
clearly
we
can't
do
that
here.
A
F
F
A
I'm
not
really
sure,
I'm
not
entirely
sure.
I
get
nervous
about
adding
keys
to
my
keychain,
because
sometimes
I
drop
and
lose
my
keys.
I
misplace
them
yeah.
You
asked
a
great
question
about
just
general
best
practices
and
I
wanna.
Maybe
we
can
talk
about
it
for
just
a
couple
more
minutes.
I'm
gonna
do
a
a
shameless
plug
of
a
communication,
best
practice
that
has
really
revolutionized
how
I
communicate
on
mrs
called
conventional
comments.
A
So
if
I
notice
user
facing
issues
I'll
leave
these
issue
labels
and
stuff,
so
I
describe
a
handful
of
labels
that
I
find
helpful
and
I've
seen
used
in
the
wild.
So
you
might
see
that
happening
in
mrs,
u
reveal
or
author,
but
this
has
been
really
helpful
for
me
because
I'm
also
don't
like
to
be
thinking
about
how
I'm
going
to
phrase
something
a
lot.
So
this
actually
automates
a
lot
of
that
thought
process
of
how
I
want
to
phrase
something
are.
B
B
B
A
G
G
D
A
A
We
have
a
browser
extension,
that's
for
conventional
comments.
We
have
one
like.
I
would
just
suggest
if
you're
gonna
do,
that,
I
think
everyone's
has
their
own.
This
is
you're
getting
to
personal
philosophy,
but
like
we
have
our
own,
that
is
open
source
and
so
that's
a
little
nice.
But
it
is
scary
when
you
install
browser
extensions
and
they
want
all
sorts
of
privileges
like
we'd
like
to
you
know,
we'd
like
to
deposit
to
your
checking
account
like
that's
a
big
red
flag.
A
B
A
Cool
does
even
have
anything
else,
they'd
like
to
add
to
the
show
and
tell
their
questions.
Part
of
this
session
coming
up
is
pair,
reviewing
which
I'll
be
eager
to
get
to,
because
we
didn't
really
get
to
have
a
whole
lot
of
time
last
time,
but
if
someone
does
have
something
that
would
be
valuable.
Let's,
let's
talk
about
it,
sure.
A
All
right,
let's,
let's
jump
into
some,
let's
jump
into
some
code
reviews.
Does
anyone
have
any,
mrs
that
they
would
like
to?
Maybe
we
could
all
mob
pair
review
on
it.
So,
if
you're
not
familiar
with
how
we
do
pair
programming
on
our
back
end
or
front-end
pair
programming
sessions,
usually
there's
one
person
driving
and
they
share
their
screen
and
then
the
rest
are
observing
navigating
discussing-
and
it's
usually
surprisingly,
works
really
well.
A
So
it's
kind
of
this
mob
pairing
thing
and
it's
facilitates
some
really
good
collaboration
and
knowledge
sharing
so
yeah
I
have
some
mmrs,
but
I
don't
wanna,
I
don't
wanna
be
so
attention
seeking
which
I
tend
to
be.
Oh,
a
database,
mr
in
the
future,
yeah,
that's
a
great
idea,
terry,
so
yeah.
If
you
get
painted
on
a
database,
mr
on.
F
C
C
A
A
A
A
F
A
To
have
a
really
simple
scope
and
it's
like
12
files
changed
make
me
like
so
nervous.
Okay,
so
I
see
we
have
this
slot,
which
we
don't
have
to
worry
about
the
specifics
of,
but
it
looks
like
we
are
simply
wrapping
this
slot.
That
was
previously
there.
Where
that
runner
name,
I
guess,
will
get
injected
around
with
a
strong
element,
and
we
have
moved
this
runner
badge
over.
A
A
So
I
always
feel
funny
about
this
from
the
front
end
because
browsers
are
pretty
forgiving,
but
there
is
like
an
html
specification
that
certain
elements
can
only
have
certain
kinds
of
children
and
strong
elements
can
only
have
inline
children
and
given
that
this
is
a
slot
previously,
that
slot
could
be
anything
could
be
a
div
span
whatever,
which
is
part
of
this
component's
interface,
but
now
we're
kind
of
limiting
the
scope
of
what
this.
What
this
slot
can
do,
because
technically
it
should
only
be
probably
a
span
or
even
just
raw
text.
A
On
top
of
that,
this
happens
so
often,
especially
with
backhanders
the
right
front.
End
code,
I'm
sorry,
backhanders,
I'm
picking
on
you
all
strong,
so
there's
a
whole
semantic
to.
I
guess
I'm
not!
I
don't
know
if
this
is
still
a
thing,
but
like
semantic
html
was
we
tried.
We
tried
to
prioritize
it
sometime.
Strong
also
means
like
urgent,
like
your.
Your
child
is
sick
urgent.
A
So
we
use
it
all
the
time.
Just
a
bold
text,
it's
not
really
what
it's
meant
for
so
this
is.
This
is
great.
I
I
feel
like
it's
all
going
to
work.
It
clearly
showed
screenshots
that
it's
going
to
work,
but
it's
like
there's
other
ways
to
do
this
and,
like
maybe
we're
maybe
strong,
wasn't
the
right
element
to
use
here.
A
A
G
That
was
a
really
really
good
observation,
anna,
because
if
it
is
only
used,
one
place
you're,
keeping
it
more
cohesive
and
less
coupled
to
this
component
and
then
in
the
future.
If
this
perhaps
is
used
somewhere
else,
it
will
and
it
will
still
be
bold
like
does
the
bowl
belong
with
whatever
is
in
this
slot
or
does
the
ball
belong
here?.
A
F
A
My
life,
especially
when
you
have
the
gdk
running
cool
so
yeah
here
we're
wrapping
a
strong,
let's
check
out
runner
name
and
see
where
that's
used,
so
runner
name,
seeing
where
this
is
all
imported.
So
it
comes
from
runner
summary
cell
and
admin
runners,
app
and
group
runners
app,
so
admin
runners,
app.
A
A
Yeah,
so
for
me
personally,
I
would
leave
this
as
since
there's
nothing
urgent
here
and
I'm
like,
let's
not
use,
misused,
strong,
I'm
leaving
a
polish
comment
of
like
hey,
let's
just
polish
it
up.
This
is
the
first
mr
cycle.
This
is
the
only
small
thing
I
see
it's
like
this
yeah
we
like
to
merge
things,
but
let's
just
polish
it
up
a
little
bit.
I'm
saying
that
strong.
G
G
A
A
G
A
A
G
A
F
A
E
C
G
Added
the
ui
element
with
it
in
the
right
with
the
the
treatment,
but
then
they
abandoned
that
feature
and
they're
not
doing
that.
So
now
I
think
they're
iterating
on
other
aspects
of
the
design
of
more
like
the
workflow
in
different
stages
to
you
know
whether
you're
requested
or
re-requested,
and
who
has
the
responsibility
for
it.
C
C
C
C
A
G
That's
the
feedback
I
was
giving
to
them
yesterday
as
part
of
that
their
work
on
this
I
was
like.
I
have
an
email,
centric
workflow
and
I
have
a
like
an
automated
job
that
gives
it
a
big
red
label
that
says
reviewer
when,
when
that
special
header
is
in
the
email
contents
down
which
works
most
of
the
time,
but
like
that
doesn't
work
well
for
unassigning
and
reassigning
myself,
then
I
have
to
go
in
and
watch
this,
which
is
that's
two
places
to
look
yeah.
A
A
This
one
is
interesting.
This
is
working
with
an
interesting
part
of
our
app.
Is
this
the
one
that
I'm
thinking
of?
Oh?
No,
I
think
this
no.
I
was
thinking
a
different
one.
That
one
is
interesting
all
all
mrs!
I
get
are
interesting
that
wasn't
the
one
I
was
thinking
of,
though
this
one
is
about
adding
some
tracking
to
our
pipeline
wizard,
so
we're
having
a.
I
think,
it's
behind
a
feature
flag,
but
it's
a
it's
a
wizard
for
creating
the
yaml
needed
for
paige's
pipeline.
A
A
A
F
F
A
A
A
Yeah
I
got,
I
got
a
fancy.
I
got
a
fancy
computer
though
right
my
fancy
computer
told
me
it
can
it
has.
I
could
multitask,
unlike
myself,
so
what
I
do
then.
So
this
is
the
unit
test.
I'm
running
and
I
will
just
straight
up
like
change
I'll
see
what
lines
we're
changing
and
I'll
actually
just
introduce
bugs
like.
A
Yeah,
so
this
is
actually
called.
This
is
called
mutation
testing
and
what
you're
doing
is
you're
introducing
mutants
into
the
subject
or
the
production
code,
and
if
the
tests
pass,
your
coverage
fails.
So
if
I
can
introduce
bugs
and
the
tests
still
pass,
I
have
not
adequate
tests.
So
I
will
do
that.
That's
my
favorite
way
to
test
our
test
coverage
is
I'll,
admit,
looking
at
the
lines
that
are
changed.
A
A
Just
at
the
unit
test
level
of
me
knowing
what's
covered
and
what's
not
so
you
could
do
the
same
thing
in
the
back
end
code
of
changing
a
function
to
throw
an
error
or
be
erroneous
values
or
something,
and
if
this
would
cause
a
bug
and
our
tests
aren't
failing.
That's
a
really
good
signal.
If
our
coverage
isn't
good
enough.
G
And
so
wouldn't
be
a
call
with
me
and
paul
on
it
if
I
did
not
say
this,
but
the
jetbrains
ide
in
ruby
mine,
which
I
use
when
you
run
your
test.
If
you
right
click,
there's
an
option
to
run
with
coverage
and
then
it
will
automatically
highlight
red
and
green
the
lines
which
are
covered
and
which
are
not
that's
fancy
yeah.
F
C
C
B
G
A
A
F
D
A
A
A
A
G
D
A
G
F
A
A
A
I
am
compelled
to
want
to
just
double
check
it.
It
looks
like
the
other
significant
change
to
this.
Mr
is
just
these
doc
changes
where
it's
hard
to
tell
what's
going
on
in
this
kind
of
diff.
But
if
I
go
here,
we're
just
adding
a
lot
of
white
space,
but
adding
this
documentation
to
these
yaml
interfaces
that
he's
had
before.
A
I
think
I
think
that's
really
the
only
to
do
left
for
me
on
this.
Mr,
is.
I
want
to
verify
that
once
behavior,
I'm
looking
now,
I'm
just
doing
a
read-through
of
seeing
if
anything
is
really
jumping
out
of
me,
but
we're
doing
we
have
testing
for-
and
I
remember
verifying
this
earlier
and
praising
it.
I
don't
think
anything's
changed
if
I
was
really
nervous
about
something
that
I
reviewed
previously
changing.
A
A
G
A
A
A
G
That
will
determine
how
many
concurrent
reviews
you
have
assigned
to
you.
So
the
unassigning
yourself
interacts
with
that.
Like
paul
loves
reviews,
he
would
happily
do
like
50
a
day.
I
personally,
I'm,
like
you
know
couple
couple
at
a
time
is
my
max,
maybe
one
if
it's
a
bad
week.
So
if
I
remove
myself,
I'm
gonna
get
another
one.