Loading ...
Sorry, an error occurred while loading the content.
 

Re: [XP] I still hate code reviews

Expand Messages
  • David Morash
    ... Hash: SHA1 It sounds like you hate ideologues, not code reviews. :) I ve had similar experiences on my current team, and it seems to boil down to not
    Message 1 of 23 , Aug 4, 2009
      -----BEGIN PGP SIGNED MESSAGE-----
      Hash: SHA1

      It sounds like you hate ideologues, not code reviews. :)

      I've had similar experiences on my current team, and it seems to boil
      down to not having a good understanding of the other team members
      viewpoints on the "rules" and a lack of context for the code being
      reviewed. We didn't actually discuss our rules that much and that I
      think is the root of our problems.

      While we pair on a lot of code, anything we don't pair on (defect fixes
      typically) is reviewed. I had expected that after everyone having
      paired with everyone else, that we would have a good understanding of
      where each other was coming from, but while pairs seem to be more open
      to interpreting the rules in context (well they would cause they are
      there), that seems to be forgotten when it comes to reviews.

      When doing reviews I don't know if the other team member shares my
      interpretation of our rules, and vice versa. This leads to the
      explanatory lecturing you cited and generally makes reviews a drag.

      Perhaps the problem is that we aren't pairing on everything and thus the
      reviewer is lacking the context in which the code was written.



      Phlip wrote:
      > Extremists:
      >
      > I still hate code reviews. I also think code reviews are very important, and
      > other people should submit to them early and often.
      >
      > My beef is with reviewers who claim I broke some rule, when that rule was itself
      > the reason for the decision. For example, a former manager once "caught" me
      > writing a comment, and gave me The Comment Lecture. I explained that I _know_
      > The Comment Lecture by heart, and I was only using the comment to raise
      > suspicion. A while later he caught me writing yet another comment. Both comments
      > said the same thing: "This code has a fixable issue, but I'm too busy getting
      > micromanaged to do anything about it."
      >
      > Another time, I had a big graphic in a website, and I scaled it down to a
      > thumbnail by sending the entire image over the wire (it's still acceptably
      > small), and then sizing it with WIDTH= and HEIGHT= attributes.
      >
      > If this had been reviewed, the reviewer would have dinged me (doubtless in my
      > Permanent Record) for not using ImageMagick on the server to resize the image,
      > before efficiently transmitting it.
      >
      > The problem with that theory is the very next page in the website presented the
      > graphic full size. By refraining from screwing with the graphic, I was
      > exploiting the browser's image cache to make _both_ pages more efficient.
      >
      > In conclusion, as Miss America lisped in the movie "Bananas", when Woody Allen
      > was on trial for being un-American, "Differences of opinion should be tolerated
      > in a democracy - but not when they're /too/ different!"
      >

      -----BEGIN PGP SIGNATURE-----
      Version: GnuPG v1.4.9 (GNU/Linux)
      Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

      iEYEARECAAYFAkp4cvYACgkQV8hFFknmM/vyJACdFy+3GJKKmfuB/I6aKueaYsPi
      PloAoJG/3Ca3nqzMNbLTwtAa+/sbCUjq
      =kA8N
      -----END PGP SIGNATURE-----
    • Phlip
      ... Okay - code review early and often. I know you didn t say this, but only code reviewing after a bug bites will set up negative rewards...
      Message 2 of 23 , Aug 4, 2009
        > While we pair on a lot of code, anything we don't pair on (defect fixes
        > typically) is reviewed.

        Okay - code review early and often. I know you didn't say this, but only
        code reviewing after a bug bites will set up negative rewards...
      • David Morash
        ... For defects we generally write tests to catch the defect, then fix it refactoring as needed, prior to having both, the fix and tests reviewed. I don t
        Message 3 of 23 , Aug 4, 2009
          Phlip wrote:
          >> While we pair on a lot of code, anything we don't pair on (defect fixes
          >> typically) is reviewed.
          >
          > Okay - code review early and often. I know you didn't say this, but only
          > code reviewing after a bug bites will set up negative rewards...

          For defects we generally write tests to catch the defect, then fix it
          refactoring as needed, prior to having both, the fix and tests reviewed.

          I don't quite follow, are you suggesting we should code review more
          often versus pairing more often?
        • Phlip
          ... When pairing often, bugs cause code reviews. These reviews now have teeth - they are not about coding aphorisms, they are about details that actually went
          Message 4 of 23 , Aug 4, 2009
            David Morash wrote:

            > I don't quite follow, are you suggesting we should code review more
            > often versus pairing more often?

            When pairing often, bugs cause code reviews. These reviews now have teeth - they
            are not about coding aphorisms, they are about details that actually went wrong.

            If the general code reviewing hurts, however, do it more often.

            --
            Phlip
          • Bill Caputo
            ... And when code reviews are about aphorisms, they re really about those in the team enforcing (dictating?) team culture to those just currently on the
            Message 5 of 23 , Aug 5, 2009
              On Wed, Aug 5, 2009 at 12:28 AM, Phlip<phlip2005@...> wrote:
              > When pairing often, bugs cause code reviews. These reviews now have teeth - they
              > are not about coding aphorisms, they are about details that actually went wrong.

              And when code reviews are about aphorisms, they're really about those
              "in the team" enforcing (dictating?) team culture to those just
              currently "on the team." And while I freely admit that code reviews
              are useful, I hate them too - when they too often take this toothless
              (from a quality perspective) form.

              Incidentally, I am not passing judgment one way or the other on the
              "in-teaming" behavior per se (I think it's inevitable in some form);
              it's just that via code reviews, its very inefficient and often
              produces reactions such as Phlip is describing - which ironically
              undermines team cohesion (not that this deters those who love to get
              your code up on a projector and berate you about your adherence to the
              coding standard in front of an admiring crowd).

              Best,
              Bill
            • Phlip
              ... Ding! (BTW, nothing personal, but Kanban is on my must eternally ignore and never learn what it is list... Hence the distraction!)
              Message 6 of 23 , Aug 5, 2009
                Bill Caputo wrote:

                > And when code reviews are about aphorisms, they're really about those
                > "in the team" enforcing (dictating?) team culture to those just
                > currently "on the team."

                Ding!

                (BTW, nothing personal, but Kanban is on my "must eternally ignore and never
                learn what it is" list... Hence the distraction!)
              • Chris Wheeler
                ... Ya, I m sure it was the rules you broke, and not the content of your comment, that pissed off your manager. Chris. [Non-text portions of this message have
                Message 7 of 23 , Aug 5, 2009
                  On Mon, Aug 3, 2009 at 1:12 AM, Phlip <phlip2005@...> wrote:

                  > . A while later he caught me writing yet another comment. Both comments
                  > said the same thing: "This code has a fixable issue, but I'm too busy
                  > getting
                  > micromanaged to do anything about it."
                  >


                  Ya, I'm sure it was the rules you broke, and not the content of your
                  comment, that pissed off your manager.

                  Chris.


                  [Non-text portions of this message have been removed]
                • Phlip
                  ... Of course I didn t actually say that. And you have been scornful of me for a while here - please look to your own behavior!
                  Message 8 of 23 , Aug 5, 2009
                    >> . A while later he caught me writing yet another comment. Both comments
                    >> said the same thing: "This code has a fixable issue, but I'm too busy
                    >> getting
                    >> micromanaged to do anything about it."
                    >
                    > Ya, I'm sure it was the rules you broke, and not the content of your
                    > comment, that pissed off your manager.
                    >
                    > Chris.

                    Of course I didn't actually say that. And you have been scornful of me for a
                    while here - please look to your own behavior!
                  • Villanueva, Vincent
                    yes, i really like it as well. thanks, vince ________________________________ From: extremeprogramming@yahoogroups.com
                    Message 9 of 23 , Aug 5, 2009
                      yes, i really like it as well.
                      thanks,
                      vince

                      ________________________________

                      From: extremeprogramming@yahoogroups.com
                      [mailto:extremeprogramming@yahoogroups.com] On Behalf Of Phlip
                      Sent: Wednesday, August 05, 2009 12:18 PM
                      To: extremeprogramming@yahoogroups.com
                      Subject: [XP] Re: I still hate code reviews




                      >> . A while later he caught me writing yet another comment. Both
                      comments
                      >> said the same thing: "This code has a fixable issue, but I'm too busy
                      >> getting
                      >> micromanaged to do anything about it."
                      >
                      > Ya, I'm sure it was the rules you broke, and not the content of your
                      > comment, that pissed off your manager.
                      >
                      > Chris.

                      Of course I didn't actually say that. And you have been scornful of me
                      for a
                      while here - please look to your own behavior!






                      [Non-text portions of this message have been removed]
                    • Ron Jeffries
                      Hello, Chris. On Wednesday, August 5, 2009, at 2:13:11 PM, you ... If I didn t know you so well, I d have read that as sarcastic ... Ron Jeffries
                      Message 10 of 23 , Aug 5, 2009
                        Hello, Chris. On Wednesday, August 5, 2009, at 2:13:11 PM, you
                        wrote:

                        > Ya, I'm sure it was the rules you broke, and not the content of your
                        > comment, that pissed off your manager.

                        If I didn't know you so well, I'd have read that as sarcastic ...

                        Ron Jeffries
                        www.XProgramming.com
                        www.xprogramming.com/blog
                        We know less about the project today than at any time in the future.
                        -- Chet Hendrickson
                        You mean today is the dumbest day of the rest of my life?
                        -- Ron Jeffries
                      • Chris Wheeler
                        ... I m not scornful of you: your posts are often left of centre and slightly bizarre, and my brain has a hard time interpreting what you are saying. And I
                        Message 11 of 23 , Aug 5, 2009
                          On Wed, Aug 5, 2009 at 3:18 PM, Phlip <phlip2005@...> wrote:

                          > >> . A while later he caught me writing yet another comment. Both comments
                          > >> said the same thing: "This code has a fixable issue, but I'm too busy
                          > >> getting
                          > >> micromanaged to do anything about it."
                          > >
                          > > Ya, I'm sure it was the rules you broke, and not the content of your
                          > > comment, that pissed off your manager.
                          > >
                          > > Chris.
                          >
                          > Of course I didn't actually say that. And you have been scornful of me for
                          > a
                          > while here - please look to your own behavior!
                          >

                          I'm not scornful of you: your posts are often left of centre and slightly
                          bizarre, and my brain has a hard time interpreting what you are saying. And
                          I *did* look to my own behaviour - I've written very similar things in code
                          comments in the past - including, but not limited to orders for pizza and
                          reviews of Van Halen albums - and so I didn't think it out of the realm of
                          possibility that you would have done the same thing!

                          It would have been wrong for me to pin bad-manager behaviours on my boss for
                          criticizing my comments.

                          Chris.


                          [Non-text portions of this message have been removed]
                        • David Morash
                          ... Hash: SHA1 ... Okay, I follow. For us, bugs don t trigger code reviews, they do trigger an analysis of the cause and may result in additional work to
                          Message 12 of 23 , Aug 5, 2009
                            -----BEGIN PGP SIGNED MESSAGE-----
                            Hash: SHA1

                            Phlip wrote:
                            > David Morash wrote:
                            >
                            >> I don't quite follow, are you suggesting we should code review more
                            >> often versus pairing more often?
                            >
                            > When pairing often, bugs cause code reviews. These reviews now have teeth - they
                            > are not about coding aphorisms, they are about details that actually went wrong.
                            >
                            > If the general code reviewing hurts, however, do it more often.
                            >

                            Okay, I follow. For us, bugs don't trigger code reviews, they do
                            trigger an analysis of the cause and may result in additional work to
                            "fix" other areas but the team members just do this, there is no
                            enforcement.

                            I can't see how a review would be more effective than simply fixing the
                            defect and other areas suffering the same problem AND communicating that
                            to the team. I don't see why a team would choose reviews over that,
                            unless they really didn't trust each others abilities.

                            Now I have been on teams where I didn't trust the other team members
                            abilities and vice versa, but reviews don't fix that.
                            -----BEGIN PGP SIGNATURE-----
                            Version: GnuPG v1.4.7 (MingW32)
                            Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

                            iD8DBQFKeiMjV8hFFknmM/sRAotiAKC+BQ3eaF5Am8Ax5dOMhyVjIUGHOwCgijov
                            mUI3+l8XBQsjuR0duasCbRo=
                            =O6/0
                            -----END PGP SIGNATURE-----
                          • David Morash
                            ... Hash: SHA1 ... Maybe everyone could just assume good intent on the part of others? ... Version: GnuPG v1.4.7 (MingW32) Comment: Using GnuPG with Mozilla -
                            Message 13 of 23 , Aug 5, 2009
                              -----BEGIN PGP SIGNED MESSAGE-----
                              Hash: SHA1

                              Ron Jeffries wrote:
                              > Hello, Chris. On Wednesday, August 5, 2009, at 2:13:11 PM, you
                              > wrote:
                              >
                              >> Ya, I'm sure it was the rules you broke, and not the content of your
                              >> comment, that pissed off your manager.
                              >
                              > If I didn't know you so well, I'd have read that as sarcastic ...
                              >

                              Maybe everyone could just assume good intent on the part of others?
                              -----BEGIN PGP SIGNATURE-----
                              Version: GnuPG v1.4.7 (MingW32)
                              Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

                              iD8DBQFKeiMtV8hFFknmM/sRAoLlAKCRlHanQoCcRbcWX1qYVP7ZqO2wmQCcCV9f
                              sJuS1UlbGuZGBAYAo2uruhw=
                              =ebjj
                              -----END PGP SIGNATURE-----
                            • Adam Sroka
                              On Wed, Aug 5, 2009 at 8:04 AM, Bill ... Right. It s a bully pulpit. There is no value in that. The point of sharing your code and your insights with one
                              Message 14 of 23 , Aug 5, 2009
                                On Wed, Aug 5, 2009 at 8:04 AM, Bill
                                Caputo<list-subscriber@...> wrote:
                                >
                                >
                                > On Wed, Aug 5, 2009 at 12:28 AM, Phlip<phlip2005@...> wrote:
                                >> When pairing often, bugs cause code reviews. These reviews now have teeth
                                >> - they
                                >> are not about coding aphorisms, they are about details that actually went
                                >> wrong.
                                >
                                > And when code reviews are about aphorisms, they're really about those
                                > "in the team" enforcing (dictating?) team culture to those just
                                > currently "on the team." And while I freely admit that code reviews
                                > are useful, I hate them too - when they too often take this toothless
                                > (from a quality perspective) form.
                                >

                                Right. It's a bully pulpit. There is no value in that. The point of
                                sharing your code and your insights with one another is to facilitate
                                communication. These sorts of code reviews are likely to have the
                                opposite effect.

                                > Incidentally, I am not passing judgment one way or the other on the
                                > "in-teaming" behavior per se (I think it's inevitable in some form);
                                > it's just that via code reviews, its very inefficient and often
                                > produces reactions such as Phlip is describing - which ironically
                                > undermines team cohesion (not that this deters those who love to get
                                > your code up on a projector and berate you about your adherence to the
                                > coding standard in front of an admiring crowd).
                                >

                                If we are pair programming and sitting together then why can't we just
                                bring it up when we see it? "Hey, this code stinks!" And have the
                                conversation then. Otherwise, when we have a need to change it we just
                                change it. Why beat a dead horse?
                              • Adam Sroka
                                ... Crazy talk ;-)
                                Message 15 of 23 , Aug 5, 2009
                                  On Wed, Aug 5, 2009 at 5:26 PM, David Morash<davidmorash@...> wrote:
                                  >
                                  >
                                  > -----BEGIN PGP SIGNED MESSAGE-----
                                  > Hash: SHA1
                                  >
                                  > Ron Jeffries wrote:
                                  >> Hello, Chris. On Wednesday, August 5, 2009, at 2:13:11 PM, you
                                  >> wrote:
                                  >>
                                  >>> Ya, I'm sure it was the rules you broke, and not the content of your
                                  >>> comment, that pissed off your manager.
                                  >>
                                  >> If I didn't know you so well, I'd have read that as sarcastic ...
                                  >>
                                  >
                                  > Maybe everyone could just assume good intent on the part of others?

                                  Crazy talk ;-)
                                • Jeff Grigg
                                  ... I have to admit that the original post made me think that the actual comment in the actual code *DID* say something like that. ... Me too. It was a bad
                                  Message 16 of 23 , Aug 5, 2009
                                    >>> --- Phlip <phlip2005@...> wrote:
                                    >>>> [...] Both comments said the same thing: "This code has a
                                    >>>> fixable issue, but I'm too busy getting micromanaged to do
                                    >>>> anything about it."

                                    >> -- Chris.
                                    >>> Ya, I'm sure it was the rules you broke, and not the content
                                    >>> of your comment, that pissed off your manager.

                                    > --- Phlip <phlip2005@...> wrote:
                                    >> Of course I didn't actually say that. [...]

                                    --- Chris Wheeler <christopher.wheeler@...> wrote:
                                    > [...] my brain has a hard time interpreting what you are saying.

                                    I have to admit that the original post made me think that the actual comment in the actual code *DID* say something like that.

                                    > [...] - I've written very similar things in code comments
                                    > in the past - [...] - and so I didn't think it out of the
                                    > realm of possibility that you would have done the same thing!

                                    Me too. It was a bad idea. And she caught me. Very embarrassing.


                                    > It would have been wrong for me to pin bad-manager behaviors
                                    > on my boss for criticizing my comments.

                                    "So you want me to get rid of the TODO comments? Really? Well, OK, but you realize that I'll have to ***DO*** what it says in the TODO comments -- and that that may take a while. As long as you're OK with the cost and the schedule delay, I'm *always* up for doing more work. Honestly, I am; I just thought that it wouldn't provide sufficient ROI at this time -- that's why I deferred it. But if you want that work done now, I'd be glad to do it."

                                    On the other hand, if it's a "this is why it's like this" comment, I'd ask what they'd like me to do -- other than just to delete the comment.
                                  • Adam Sroka
                                    ... ... FWIW, there is almost always a better place to put TODO comments than as comments in the code. For example both Perl and Ruby support TODO
                                    Message 17 of 23 , Aug 5, 2009
                                      On Wed, Aug 5, 2009 at 7:34 PM, Jeff Grigg<jeffgrigg@...> wrote:
                                      >
                                      < snip >

                                      > "So you want me to get rid of the TODO comments? Really? Well, OK, but you
                                      > realize that I'll have to ***DO*** what it says in the TODO comments -- and
                                      > that that may take a while. As long as you're OK with the cost and the
                                      > schedule delay, I'm *always* up for doing more work. Honestly, I am; I just
                                      > thought that it wouldn't provide sufficient ROI at this time -- that's why I
                                      > deferred it. But if you want that work done now, I'd be glad to do it."
                                      >

                                      FWIW, there is almost always a better place to put "TODO" comments
                                      than as comments in the code. For example both Perl and Ruby support
                                      "TODO" in a test (Perl has TODO {} blocks via Test::Harness. RSpec has
                                      pending().) I don't like TODO comments in code for the same reason I
                                      don't like comments generally: they don't help me understand the code
                                      and they have no executable semantics.

                                      > On the other hand, if it's a "this is why it's like this" comment, I'd ask
                                      > what they'd like me to do -- other than just to delete the comment.
                                      >
                                      >
                                    • George Dinwiddie
                                      ... TODO comments work pretty well in Eclipse, where they appear in the task list. At least, they work pretty well if you don t let them persist. I like to
                                      Message 18 of 23 , Aug 5, 2009
                                        Adam Sroka wrote:
                                        > FWIW, there is almost always a better place to put "TODO" comments
                                        > than as comments in the code. For example both Perl and Ruby support
                                        > "TODO" in a test (Perl has TODO {} blocks via Test::Harness. RSpec has
                                        > pending().) I don't like TODO comments in code for the same reason I
                                        > don't like comments generally: they don't help me understand the code
                                        > and they have no executable semantics.

                                        TODO comments work pretty well in Eclipse, where they appear in the task
                                        list. At least, they work pretty well if you don't let them persist. I
                                        like to make them all go away before calling the story done. With
                                        legacy code, that's not always possible (and I'll generally use a
                                        different task tag, such as SMELL).

                                        - George

                                        --
                                        ----------------------------------------------------------------------
                                        * George Dinwiddie * http://blog.gdinwiddie.com
                                        Software Development http://www.idiacomputing.com
                                        Consultant and Coach http://www.agilemaryland.org
                                        ----------------------------------------------------------------------
                                      • Ron Jeffries
                                        Hello, David. On Wednesday, August 5, 2009, at 8:26:21 PM, you ... Were you modeling that behavior just then? :) Ron Jeffries www.XProgramming.com
                                        Message 19 of 23 , Aug 5, 2009
                                          Hello, David. On Wednesday, August 5, 2009, at 8:26:21 PM, you
                                          wrote:

                                          >>> Ya, I'm sure it was the rules you broke, and not the content of your
                                          >>> comment, that pissed off your manager.
                                          >>
                                          >> If I didn't know you so well, I'd have read that as sarcastic ...

                                          > Maybe everyone could just assume good intent on the part of others?

                                          Were you modeling that behavior just then? :)

                                          Ron Jeffries
                                          www.XProgramming.com
                                          www.xprogramming.com/blog
                                          Analysis kills spontaneity.
                                          The grain once ground into flour germinates no more. -- Henri Amiel
                                        • Adam Sroka
                                          ... True, but they also show up if you put them in the tests ;-) You can also create task lists independent of the code in Eclipse. Or, something I have done
                                          Message 20 of 23 , Aug 5, 2009
                                            On Wed, Aug 5, 2009 at 8:04 PM, George Dinwiddie<lists@...> wrote:
                                            >
                                            >
                                            > Adam Sroka wrote:
                                            >> FWIW, there is almost always a better place to put "TODO" comments
                                            >> than as comments in the code. For example both Perl and Ruby support
                                            >> "TODO" in a test (Perl has TODO {} blocks via Test::Harness. RSpec has
                                            >> pending().) I don't like TODO comments in code for the same reason I
                                            >> don't like comments generally: they don't help me understand the code
                                            >> and they have no executable semantics.
                                            >
                                            > TODO comments work pretty well in Eclipse, where they appear in the task
                                            > list. At least, they work pretty well if you don't let them persist. I
                                            > like to make them all go away before calling the story done. With
                                            > legacy code, that's not always possible (and I'll generally use a
                                            > different task tag, such as SMELL).
                                            >

                                            True, but they also show up if you put them in the tests ;-) You can
                                            also create task lists independent of the code in Eclipse. Or,
                                            something I have done in the past is to create my own @Todo annotation
                                            class to use with JUnit tests.
                                          • David Morash
                                            ... Hash: SHA1 ... I hope so! Sorry, that wasn t directed at any one person (but I didn t make that clear). I just felt like there was bit of tension at parts
                                            Message 21 of 23 , Aug 5, 2009
                                              -----BEGIN PGP SIGNED MESSAGE-----
                                              Hash: SHA1

                                              Ron Jeffries wrote:
                                              > Hello, David. On Wednesday, August 5, 2009, at 8:26:21 PM, you
                                              > wrote:
                                              >
                                              >>>> Ya, I'm sure it was the rules you broke, and not the content of your
                                              >>>> comment, that pissed off your manager.
                                              >>> If I didn't know you so well, I'd have read that as sarcastic ...
                                              >
                                              >> Maybe everyone could just assume good intent on the part of others?
                                              >
                                              > Were you modeling that behavior just then? :)

                                              I hope so!

                                              Sorry, that wasn't directed at any one person (but I didn't make that
                                              clear). I just felt like there was bit of tension at parts of this
                                              thread, but to quote a useful phrase I've learned here, "I could be
                                              wrong. I often am."

                                              -----BEGIN PGP SIGNATURE-----
                                              Version: GnuPG v1.4.9 (GNU/Linux)
                                              Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

                                              iEYEARECAAYFAkp6VdQACgkQV8hFFknmM/v2UACgveFxwyph26tntkTQ1G8iIoT9
                                              LQMAoL2/eHeRImoz2l10sZAxtDJEL2GO
                                              =rmcJ
                                              -----END PGP SIGNATURE-----
                                            • Tim Ottinger
                                              Sometimes the rules need to be changed.Maybe the OP should consider using the next retrospective as a place & time to change the rules. Of course I think that
                                              Message 22 of 23 , Aug 11, 2009
                                                Sometimes the rules need to be changed.Maybe the OP should consider using the next retrospective as a place & time to change the rules.
                                                Of course I think that my rules on comments are the most reasonable: http://agileinaflash.blogspot.com/2009/04/rules-for-commenting.html

                                                I used to teach code review techniques, and I believed very strongly in their use. Then I got over it somehow.

                                                I've only seen reviews remain profitable in non-agile teams. In a proper agile team with no silos and promiscuous pairing, one may find some issues, but at a cost greater than the benefit received. Understandably life-critical systems are special. Is the code review a poor substitute for having good ATs/UTs and more eyes on-task? Would you catch anything in the code review that shouldn't be caught in a test?

                                                tim
                                              Your message has been successfully submitted and would be delivered to recipients shortly.