Press "Enter" to skip to content

Code review discipline and working contracts – MPJ’s Musings Fun Fun Function


good Monday morning today I am taking

some questions from the fan fan forum

program artist how to manage pull

requests in a team how to maintain pull

requests pile as low as possible while

keeping the team focus high from one

hand if you want it to be on one hand

you want it to breed be reviewed as

quickly as possible from the air and on

the other hand you don’t want to create

too many context switches for your team

members yeah getting a good pull request

code review culture in a team is the

it’s a bit tricky to establish it

doesn’t come naturally you have to spend

a little bit of work like so you just

just be patient with this it takes a

while but that said in the last team I

was in on at Spotify on the desktop team

we we had an agile coach Jimmy which was

fantastic and he was big on this thing

called a working contract and a working

contract is something that you develop

in a meeting it’s usually in the form of

a post-it note and it says something

that the team agrees to do it’s not a

task it’s more like a habit so for

instance in this case the working

agreement was I do code reviews every

day that was the working contract and in

the thing with the working contract is

that every single person in the room

must agree that this is a priority task

and that this is something that we do or

you don’t put that working contract up

it needs to be completely unanimous if

someone is is on the fence you need to

convince that person or if that person

is not convinced you put that work a

contract thing and into like this maybe

column where you where well we’re gonna

perhaps try it but not

everybody’s convinced that this is a

good idea so it’s very much in trial

mode and we’re just observing it for

instance I don’t remember any that was

in the column hmm off the top of my head

but either way the important thing is

that it has to be unanimous and as the

week’s go by like after a week or two

you start putting like checking going

through the working contracts and seeing

which of these are we actually doing for

instance you might do like we always

write unit tests for our code might be a

working contract and you just observe

that you realize that and it doesn’t

really work with old ways we figured out

that hmm we had to there are some pieces

of the code that just isn’t suitable for

unit tests or it’s just an insane

bizarre amount of work to get this part

of the code under test so we have

figured that no we’re not really

adhering to this contract so we decide

either that we double down on it and

focus on it or we decide to make it more

realistic so that it actually

corresponds to the working process that

we have that case we decided to change

it to write unit tests or write a very

good excuse so that yeah I always add

unit tests or have a very good excuse

and that turned out to be a really good

balance because then then people can

challenge that excuse a little bit

Justin that you you avoid the case where

you just don’t write the unit test out

of lethargy and but it also allowed us

to skip unit tests for the cases where

the cost was simply too high but going

back to the the case where with the

palookas so the working contract there

that we decided upon

was code review every day so that means

that you realistically will never have

to wait more than a day for your code

review to be to be completed often a lot

less because everybody reviews all the

time and some people would like to put

their review block in the morning some

will you do it just because before they

go home so like they became this pretty

nice distribution and you you got your

code reviewed in a pretty expedient

manner I think it turned out to be

pretty nice it allowed also allowed

everyone to build a routine around the

work because they they could choose a

time of day where it was suitable for

them to do code review like in the

morning or in the afternoon and it also

allowed you to it kind of become became

this thing where you slammed in your

thing and somewhere during the day and

then you just okay there’s gonna be a

like it’s gonna take a day before I get

this back so now I go do something else

so just just send them away for this

feedback loop and then I do something

else it required some context switching

of course that’s that’s the nature of

pull requests the only other real

alternative to do – that is – is to do

pair programming which is something that

I I love but it’s it also has some other

logistical problems and and culture

problems it’s not it’s it’s not as

trivial to get working in a team as as

pull requests or but but it’s great if

you can if you can get it if you want to

look at a company that does pair

programming well you can look at pivotal

they do it a lot they don’t even have

personal work sessions they only have

shared work stations they only do pair

programming which is cool if you try to

do it faster for instance that like

start pinging people on slack to do code

reviews you start into

rapping other people’s work and that is

that is a lot worse it creates that

creates context switching in a very bad

way and I don’t think that you should do

it if you have you’re doing pull

requests I think that was that was at

least the best the best flow that I have

seen working contract that everybody

agrees to I do code reviews every day

you have just watched an episode of fun

fun function I release these every

Monday morning Oh 800 GMT you can watch

another episode of the show by clicking

here or if you are a patron of fun fun

function you can discuss this specific

question on the Fun Fun forum by

clicking here I am npj until next Monday morning stay curious

Please follow and like us: