Wicked Uncle Nigel

The PM I'm working with (who so far seems a thoroughly decent chap[1])
is looking to implement formal peer-reviewing of code.

Does anyone have a simple protocol document that sets out a methodology
for that? I've had a rummage around my stuff, and it seems it's never
been written down.


[1] Read "My bit's ahead of schedule".
--
Wicked Uncle Nigel - "He's hopeless, but he's honest"

WS* GHPOTHUF#24 APOSTLE#14 DLC#1 COFF#20 BOTAFOT#150 HYPO#0(KoTL) IbW#41
SBS#39 OMF#6 Enfield 500 Curry House Racer "The Basmati Rice Burner",
Honda GL1000K2 (Fallen apart) Kawasaki ZN1300 Voyager "Oh, Oh, It's so big"
Suzuki TS250 "The Africa Single" Honda ST1100 wiv trailer Norton 850 Commando
Kawasaki GTR1400

Hog

Wicked Uncle Nigel wrote:
> The PM I'm working with (who so far seems a thoroughly decent chap[1])
> is looking to implement formal peer-reviewing of code.
>
> Does anyone have a simple protocol document that sets out a
> methodology for that? I've had a rummage around my stuff, and it
> seems it's never been written down.
>
>
> [1] Read "My bit's ahead of schedule".

One for Christofire I would have thought

--
Hog
'03 ST4S '96 Bastard12 '89 R100RS '81 XS650 '78 RD400

Ace

On Wed, 8 Aug 2007 18:57:34 +0100, Wicked Uncle Nigel
<wun@wicked-uncle-nigel.me.uk> wrote:

>The PM I'm working with (who so far seems a thoroughly decent chap[1])
>is looking to implement formal peer-reviewing of code.

Great idea in principle.

>Does anyone have a simple protocol document that sets out a methodology
>for that? I've had a rummage around my stuff, and it seems it's never
>been written down.

Quite a few of our users (stats programmers) started demanding such
detailed instructions when we implemented SOPs requiring what we
called Peer Quality Checks (about five years ago), but all have failed
to agree on methods for so doing, and that's within a group using only
the same environment and tools. Even getting different sites to agree
on coding standards was a hyuge struggle, only made successful by
compromise.

So we asked various teams to write the guidelines themselves, if they
were so keen to have them, and without exception they've failed to do
so. Those of us with 'proper' IT backgrounds knew that we'd always
done this and it had become semi-intuitive, and in time the others
generally began to come round to my way of thinking, which boils down
to:

"If you write down a set of rules, that's all they'll do; leaving it
ambiguous means they have to start actually engaging brain and making
informed decisions about what's acceptable in the various scenarios in
which they find themselves."

Sorry if it doesn't help; just my 2 pennorth.

--
_______
..'_/_|_\_'. Ace (brucedotrogers a.t rochedotcom)
\`\ | /`/ DS#8 BOTAFOT#3 SbS#2 UKRMMA#13 DFV#8 SKA#2 IBB#10
`\\ | //'
`\|/`
`

christofire

Hog wrote:

> Wicked Uncle Nigel wrote:
> > The PM I'm working with (who so far seems a thoroughly decent
> > chap[1]) is looking to implement formal peer-reviewing of code.
> >
> > Does anyone have a simple protocol document that sets out a
> > methodology for that? I've had a rummage around my stuff, and it
> > seems it's never been written down.
> >
> >
> > [1] Read "My bit's ahead of schedule".
>
> One for Christofire I would have thought

I've never done it formally, or to a process. He's not answering his
phone, so I can't have a quick chat to see if I know anything useful.

--
Christofire DIAABTCOD#1 DS#9 ZX-10R

Timo Geusch

Wicked Uncle Nigel <wun@wicked-uncle-nigel.me.uk> writes:

> The PM I'm working with (who so far seems a thoroughly decent chap[1])
> is looking to implement formal peer-reviewing of code.

IME, this is going to work in a spectacular fashion if it's forced upon
a development team. Please note that I'm not saying "spectacularly well".


> Does anyone have a simple protocol document that sets out a
> methodology for that? I've had a rummage around my stuff, and it seems
> it's never been written down.

I'll have a rummage through my coding and management books, I think I've
got some suggestions somewhere.

--
Morini Corsaro 125 | CB450K4 | XL250 Motosport x2 | 900SSD | K1100LT
Triumph T-Bird chop | CB400/4 BOTAFOF #33 TWA#10
The UKRM FAQ: http://www.ukrm.net/faq/index.html
"Je profite du paysage" - Joe Bar

Paul Carmichael

Bear wrote:
>
> Code works or it doesn't; the times to "review" it are a) when it's
> integrated into a build and b) when it's tested.

Early code reviews can save an immense amount of time in the long run.
Unless your testing follows every conceivable code path, and even that
won't necessarily pick up on every uninitialised variable etc., then
code reviews are absolutey essential. Don't forget, it's not just bugs
but also inefficient code (eg; look, you could unwind that loop there
and save clocks) etc.

--

Paul.
CBR1100XX SuperBlackbird (Buen mueble de patio)
And a pushbike of some sort.
BOTAFOT #4
BOTAFOF #30
MRO #24
OMF #15
UKRMMA #30

Higgins

Wicked Uncle Nigel wrote:
> The PM I'm working with (who so far seems a thoroughly decent chap[1])
> is looking to implement formal peer-reviewing of code.
You should absolutely encourage this. Think of the extra (chargeable)
days of effort that will be required.

Badger

Ace wrote:
> "If you write down a set of rules, that's all they'll do; leaving it
> ambiguous means they have to start actually engaging brain and making
> informed decisions about what's acceptable in the various scenarios in
> which they find themselves."

That's the same effect as "If you write down a set of performance
metrics, that's what they'll work to".

--
Rick
NT650V (still)
TWA#11 BREast#6 BOTAFOT#139

Timo Geusch

Paul Carmichael <arse@tit.com> writes:

> Don't forget, it's not just
> bugs but also inefficient code (eg; look, you could unwind that loop
> there and save clocks) etc.

That's something only oldtimers and/or system programmers like you and
me seem to have an interest in...

Most people these days seem to have regressed to writing code as if they
are paid per line or per class.

--
Morini Corsaro 125 | CB450K4 | XL250 Motosport x2 | 900SSD | K1100LT
Triumph T-Bird chop | CB400/4 BOTAFOF #33 TWA#10
The UKRM FAQ: http://www.ukrm.net/faq/index.html
"Je profite du paysage" - Joe Bar

Pip

On Wed, 8 Aug 2007 20:02:50 +0100, Bear <bastardDOTbear@gmail.com>
wrote:

>If you get the usual answers trotted out, simply kill the PM concerned
>with a lump of wood and bury his body somewhere where it'll rot fastest.

"Stick them in the car, and then go and look for a pig farm."

--
Pip: B12

Badger

Bear wrote:
> "Peer review of code" is a phrase tossed about by the clueless. I know,
> coz when I was clueless, I used it too.

Not necessarily true. It's a phrase we chucked around when we were
about to get rid of someone who was clueless, as part of that process.

Once he'd gone we allowed the practice to slide again.

--
Rick
NT650V (still)
TWA#11 BREast#6 BOTAFOT#139

Bear

In article <5hul61F3mada3U1@mid.individual.net>, Paul Carmichael says...
> Bear wrote:
> >
> > Code works or it doesn't; the times to "review" it are a) when it's
> > integrated into a build and b) when it's tested.
>
> Early code reviews can save an immense amount of time in the long run.
> Unless your testing follows every conceivable code path, and even that
> won't necessarily pick up on every uninitialised variable etc., then
> code reviews are absolutey essential. Don't forget, it's not just bugs
> but also inefficient code (eg; look, you could unwind that loop there
> and save clocks) etc.

"Peer review of code" is not the same thing as "module verification".
--
Bear

Wicked Uncle Nigel

Using the patented Mavis Beacon "Hunt&Peck" Technique, Higgins
<the.best.names.are.gone@gmail.com> typed
>Wicked Uncle Nigel wrote:
>> The PM I'm working with (who so far seems a thoroughly decent
>>chap[1]) is looking to implement formal peer-reviewing of code.
>You should absolutely encourage this. Think of the extra (chargeable)
>days of effort that will be required.

<whistles innocently>


--
Wicked Uncle Nigel - "He's hopeless, but he's honest"

WS* GHPOTHUF#24 APOSTLE#14 DLC#1 COFF#20 BOTAFOT#150 HYPO#0(KoTL) IbW#41
SBS#39 OMF#6 Enfield 500 Curry House Racer "The Basmati Rice Burner",
Honda GL1000K2 (Fallen apart) Kawasaki ZN1300 Voyager "Oh, Oh, It's so big"
Suzuki TS250 "The Africa Single" Honda ST1100 wiv trailer Norton 850 Commando
Kawasaki GTR1400

Wicked Uncle Nigel

Using the patented Mavis Beacon "Hunt&Peck" Technique, Ace
<seesig@virgin.net> typed
>On Wed, 8 Aug 2007 18:57:34 +0100, Wicked Uncle Nigel
><wun@wicked-uncle-nigel.me.uk> wrote:
>
>>The PM I'm working with (who so far seems a thoroughly decent chap[1])
>>is looking to implement formal peer-reviewing of code.
>
>Great idea in principle.
>
<snip)

>Sorry if it doesn't help; just my 2 pennorth.

It does, ta.

--
Wicked Uncle Nigel - "He's hopeless, but he's honest"

WS* GHPOTHUF#24 APOSTLE#14 DLC#1 COFF#20 BOTAFOT#150 HYPO#0(KoTL) IbW#41
SBS#39 OMF#6 Enfield 500 Curry House Racer "The Basmati Rice Burner",
Honda GL1000K2 (Fallen apart) Kawasaki ZN1300 Voyager "Oh, Oh, It's so big"
Suzuki TS250 "The Africa Single" Honda ST1100 wiv trailer Norton 850 Commando
Kawasaki GTR1400

Wicked Uncle Nigel

Using the patented Mavis Beacon "Hunt&Peck" Technique, christofire
<chris@ukrm.org> typed
>Hog wrote:
>
>> Wicked Uncle Nigel wrote:
>> > The PM I'm working with (who so far seems a thoroughly decent
>> > chap[1]) is looking to implement formal peer-reviewing of code.
>> >
>> > Does anyone have a simple protocol document that sets out a
>> > methodology for that? I've had a rummage around my stuff, and it
>> > seems it's never been written down.
>> >
>> >
>> > [1] Read "My bit's ahead of schedule".
>>
>> One for Christofire I would have thought
>
>I've never done it formally, or to a process. He's not answering his
>phone, so I can't have a quick chat to see if I know anything useful.

I was involved in harvesting honey.

Priorities, old chap.

I'm really just after a FFAQ, rather than ideas about whether it's a
good idea or not. I get paid either way... ;^)

--
Wicked Uncle Nigel - "He's hopeless, but he's honest"

WS* GHPOTHUF#24 APOSTLE#14 DLC#1 COFF#20 BOTAFOT#150 HYPO#0(KoTL) IbW#41
SBS#39 OMF#6 Enfield 500 Curry House Racer "The Basmati Rice Burner",
Honda GL1000K2 (Fallen apart) Kawasaki ZN1300 Voyager "Oh, Oh, It's so big"
Suzuki TS250 "The Africa Single" Honda ST1100 wiv trailer Norton 850 Commando
Kawasaki GTR1400

Wicked Uncle Nigel

Using the patented Mavis Beacon "Hunt&Peck" Technique, Timo Geusch
<tnewsSPAMMENOT@unixconsult.co.uk> typed
>Wicked Uncle Nigel <wun@wicked-uncle-nigel.me.uk> writes:
>
>> The PM I'm working with (who so far seems a thoroughly decent chap[1])
>> is looking to implement formal peer-reviewing of code.
>
>IME, this is going to work in a spectacular fashion if it's forced upon
>a development team. Please note that I'm not saying "spectacularly well".

<G>

--
Wicked Uncle Nigel - "He's hopeless, but he's honest"

WS* GHPOTHUF#24 APOSTLE#14 DLC#1 COFF#20 BOTAFOT#150 HYPO#0(KoTL) IbW#41
SBS#39 OMF#6 Enfield 500 Curry House Racer "The Basmati Rice Burner",
Honda GL1000K2 (Fallen apart) Kawasaki ZN1300 Voyager "Oh, Oh, It's so big"
Suzuki TS250 "The Africa Single" Honda ST1100 wiv trailer Norton 850 Commando
Kawasaki GTR1400

Dave Emerson

"Timo Geusch" <tnewsSPAMMENOT@unixconsult.co.uk> wrote in message
news:87odhhbxpo.fsf@odie.internal.unix-consult.com...
> Paul Carmichael <arse@tit.com> writes:
>
>> Don't forget, it's not just
>> bugs but also inefficient code (eg; look, you could unwind that loop
>> there and save clocks) etc.
>
> That's something only oldtimers and/or system programmers like you and
> me seem to have an interest in...
>
> Most people these days seem to have regressed to writing code as if they
> are paid per line or per class.

<oldtimer mode)

Like trying to get a really useful program to run in only 1k of memory on a
ZX81....

</otm>


--
Dave
ex Motorcycle Maintenance Workshop
http://tinyurl.com/4mhaw

Krusty

Bear wrote:

> In article <SPpui.3305$ka7.2187@newsfe4-gui.ntli.net>, Krusty says...
> > Bear wrote:
> >
> > > Code works or it doesn't; the times to "review" it are a) when
> > > it's integrated into a build and b) when it's tested.
> > >
> > > The question I would be asking is this: "what do you think peer
> > > review is going to give you that a decent build & test process
> > > isn't?".
> >
> > Unusually, I disagree with everything you've said. It's not about
> > whether it works or not, it's about whether it's been coded in the
> > most efficient & scalable way, or at the very least, not the most
> > inefficient & unscalable way.
>
> > There's always several ways to code something, all of which will
> > work, but only one of which will be the best way. Unfortunately the
> > best way usually isn't the laziest way, & often involves syntax
> > that wasn't in previous versions of whichever development tool is
> > being used. Therefore the developers who make a point of keeping
> > up with the changes in new releases develop more efficient code, &
> > reviews are an invaluable way of passing on that knowledge ime.
>
> Everything you've said is totally unconnected to any business case.

It's fundamental to every business case in my company.

> All that matters these days, in large-scale environments, is "does it
> work?", not "is it elegant/better than last week's solution/etc" ...
> the world view you're espousing is that of a concerned peer, not a
> business or programme manager.

It's the view of the development manager, i.e. me, & of our ops
director. I suspect it's also the view of any company who have had to
re-write code to cope with demand due to their customers increasing in
size, due to the original developers/management only worrying about
whether it works or not.


--
Krusty
www.MuddyStuff.co.uk
Off-Road Classifieds

'02 MV Senna '03 Tigtona 955i '96 Tiger '79 Fantic Hiro 250

Pip Luscher

On Wed, 8 Aug 2007 21:37:56 +0100, Wicked Uncle Nigel
<wun@wicked-uncle-nigel.me.uk> wrote:

<snip>
>Plan!

Odd. Your last three replies were to yourself according to Agent.
Either NTL has fouled up or Timo, Higgins and Carmichael are on a
swerver that's not talking to NTL.

--
-Pip

ginge

Bear wrote:
> All that matters these days, in large-scale environments, is "does it
> work?", not "is it elegant/better than last week's solution/etc" ... the
> world view you're espousing is that of a concerned peer, not a business
> or programme manager.

I'm going to disagree here, based on the customers I work with (retail,
insurance, and financial sector, mostly) there's certainly a requirement
that the code they deploy into production is efficient and scales well.
In the example of one particular high street retailer it's the
difference between taking a few hundred orders an hour, and tens of
thousands an hour, which happens almost every time they run a sale or
promotion.

Throwing more kit at the problem when they've already got several
million quids worth on the floor isn't a cost effective fix either.

I agree with you on knowledge transfer though.

platypus

Pip wrote:
> On Wed, 8 Aug 2007 20:02:50 +0100, Bear <bastardDOTbear@gmail.com>
> wrote:
>
>> If you get the usual answers trotted out, simply kill the PM
>> concerned with a lump of wood and bury his body somewhere where
>> it'll rot fastest.
>
> "Stick them in the car, and then go and look for a pig farm."

"Dad, I've crashed the Land-Rover into a pig. He's stuck under the front
bumper and squealing and making a lot of noise. I'm kinda scared. He's
really angry."

"Don't worry, son. Get out the shotgun, shoot him, back off him and drag
the carcase into the ditch, then come back home."

"Okay, Dad. I'll call you back in a couple of minutes."

"Dad, I've shot him and dragged him into the ditch, but his motorcycle is
still blocking the lane. What should I do now?"

--
platypus

somewhere to go for the night

Ace

On Wed, 08 Aug 2007 21:18:04 GMT, Pip Luscher
<pips.computer@spammers.foad.ntlworld.com> wrote:

>On Wed, 8 Aug 2007 21:37:56 +0100, Wicked Uncle Nigel
><wun@wicked-uncle-nigel.me.uk> wrote:
>
><snip>
>>Plan!
>
>Odd. Your last three replies were to yourself according to Agent.
>Either NTL has fouled up or Timo, Higgins and Carmichael are on a
>swerver that's not talking to NTL.

It's all working fine via the cheesy swerver - must be something your
end.


--
_______
..'_/_|_\_'. Ace (brucedotrogers a.t rochedotcom)
\`\ | /`/ DS#8 BOTAFOT#3 SbS#2 UKRMMA#13 DFV#8 SKA#2 IBB#10
`\\ | //'
`\|/`
`

Wicked Uncle Nigel

Using the patented Mavis Beacon "Hunt&Peck" Technique, Pip Luscher
<pips.computer@spammers.foad.ntlworld.com> typed
>On Wed, 8 Aug 2007 21:37:56 +0100, Wicked Uncle Nigel
><wun@wicked-uncle-nigel.me.uk> wrote:
>
><snip>
>>Plan!
>
>Odd. Your last three replies were to yourself according to Agent.

I get a better class of conversation that way.

--
Wicked Uncle Nigel - "He's hopeless, but he's honest"

WS* GHPOTHUF#24 APOSTLE#14 DLC#1 COFF#20 BOTAFOT#150 HYPO#0(KoTL) IbW#41
SBS#39 OMF#6 Enfield 500 Curry House Racer "The Basmati Rice Burner",
Honda GL1000K2 (Fallen apart) Kawasaki ZN1300 Voyager "Oh, Oh, It's so big"
Suzuki TS250 "The Africa Single" Honda ST1100 wiv trailer Norton 850 Commando
Kawasaki GTR1400

Simian

Bear wrote:

> In article <SPpui.3305$ka7.2187@newsfe4-gui.ntli.net>, Krusty says...
> > Bear wrote:
> >
> >
> > Unusually, I disagree with everything you've said. It's not about
> > whether it works or not, it's about whether it's been coded in the
> > most efficient & scalable way, or at the very least, not the most
> > inefficient & unscalable way.
>
> Everything you've said is totally unconnected to any business case.

Unless you're in the business of writing code that has to scale well
(like if you're google) or run in a usable fashion on a limited
platform(like if you're Nokia) or what you're doing pushes the limits
of what current hardware can do (CERN).


> All that matters these days, in large-scale environments, is "does it
> work?", not "is it elegant/better than last week's solution/etc" ...
> the world view you're espousing is that of a concerned peer, not a
> business or programme manager.

That's only the case in everyday average projects, like trivial
distributed databases or whatever.


> Knowledge transfer is better accomplished through more efficient
> means than peer review.

Only when that's possible, which is sometimes not the case, but yeah,
it _should_ be.

--
"I dunno, I never met the chick."

Bear

In article <13bkf06du8ko2f3@news.supernews.com>, Simian says...
> Bear wrote:
>
> > In article <SPpui.3305$ka7.2187@newsfe4-gui.ntli.net>, Krusty says...
> > > Bear wrote:
> > >
> > >
> > > Unusually, I disagree with everything you've said. It's not about
> > > whether it works or not, it's about whether it's been coded in the
> > > most efficient & scalable way, or at the very least, not the most
> > > inefficient & unscalable way.
> >
> > Everything you've said is totally unconnected to any business case.
>
> Unless you're in the business of writing code that has to scale well
> (like if you're google) or run in a usable fashion on a limited
> platform(like if you're Nokia) or what you're doing pushes the limits
> of what current hardware can do (CERN).

Agreed.

> > Knowledge transfer is better accomplished through more efficient
> > means than peer review.
>
> Only when that's possible, which is sometimes not the case, but yeah,
> it _should_ be.

It has to be, IMHO, IYSWIM.
--
Bear

Ofnuts

Wicked Uncle Nigel wrote:
> The PM I'm working with (who so far seems a thoroughly decent chap[1])
> is looking to implement formal peer-reviewing of code.
>
> Does anyone have a simple protocol document that sets out a methodology
> for that? I've had a rummage around my stuff, and it seems it's never
> been written down.
>
>
> [1] Read "My bit's ahead of schedule".

The peers of idiots are idiots... peer review is fundamentally flawed.
But having a knowledgeable person do code auditing can be really useful.

Bear

In article <MPG.212443dc48a9dfbe989b03@news.zen.co.uk>, ginge says...
> Bear wrote:
> > All that matters these days, in large-scale environments, is "does it
> > work?", not "is it elegant/better than last week's solution/etc" ... the
> > world view you're espousing is that of a concerned peer, not a business
> > or programme manager.
>
> I'm going to disagree here, based on the customers I work with (retail,
> insurance, and financial sector, mostly) there's certainly a requirement
> that the code they deploy into production is efficient and scales well.
> In the example of one particular high street retailer it's the
> difference between taking a few hundred orders an hour, and tens of
> thousands an hour, which happens almost every time they run a sale or
> promotion.
>
> Throwing more kit at the problem when they've already got several
> million quids worth on the floor isn't a cost effective fix either.

That maybe how your mob do it - it isn't reflected in how I see most
large-scale mobs attempt it.

Maybe it's coz I'm plugged into big programmes now, and don't think so
much in individual project terms ... Translation: the bigger they are,
the greedier they get.

> I agree with you on knowledge transfer though.

It's one of the most important things there is. Has to work, or the
programme will fail.
--
Bear

Bear

In article <Tbqui.22582$vi3.950@newsfe2-gui.ntli.net>, Krusty says...
> Bear wrote:
>
> > In article <SPpui.3305$ka7.2187@newsfe4-gui.ntli.net>, Krusty says...
> > > Bear wrote:
> > >
> > > > Code works or it doesn't; the times to "review" it are a) when
> > > > it's integrated into a build and b) when it's tested.
> > > >
> > > > The question I would be asking is this: "what do you think peer
> > > > review is going to give you that a decent build & test process
> > > > isn't?".
> > >
> > > Unusually, I disagree with everything you've said. It's not about
> > > whether it works or not, it's about whether it's been coded in the
> > > most efficient & scalable way, or at the very least, not the most
> > > inefficient & unscalable way.
> >
> > > There's always several ways to code something, all of which will
> > > work, but only one of which will be the best way. Unfortunately the
> > > best way usually isn't the laziest way, & often involves syntax
> > > that wasn't in previous versions of whichever development tool is
> > > being used. Therefore the developers who make a point of keeping
> > > up with the changes in new releases develop more efficient code, &
> > > reviews are an invaluable way of passing on that knowledge ime.
> >
> > Everything you've said is totally unconnected to any business case.
>
> It's fundamental to every business case in my company.
>
> > All that matters these days, in large-scale environments, is "does it
> > work?", not "is it elegant/better than last week's solution/etc" ...
> > the world view you're espousing is that of a concerned peer, not a
> > business or programme manager.
>
> It's the view of the development manager, i.e. me, & of our ops
> director. I suspect it's also the view of any company who have had to
> re-write code to cope with demand due to their customers increasing in
> size, due to the original developers/management only worrying about
> whether it works or not.

Out of interest, and not in the willy-waving space, what size of
projects are we talking here?

£10 million? £100 million? £1 billion?

Coz if it's anything but the first one, or below, I'd be seriously
amazed.
--
Bear

ginge

Bear wrote:
> > Throwing more kit at the problem when they've already got several
> > million quids worth on the floor isn't a cost effective fix either.
>
> That maybe how your mob do it - it isn't reflected in how I see most
> large-scale mobs attempt it.

It's actually customer driven, unsurprisingly we'd love to sell them
more kit.