Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • World
  • Users
  • Groups
Skins
  • Light
  • Cerulean
  • Cosmo
  • Flatly
  • Journal
  • Litera
  • Lumen
  • Lux
  • Materia
  • Minty
  • Morph
  • Pulse
  • Sandstone
  • Simplex
  • Sketchy
  • Spacelab
  • United
  • Yeti
  • Zephyr
  • Dark
  • Cyborg
  • Darkly
  • Quartz
  • Slate
  • Solar
  • Superhero
  • Vapor

  • Default (No Skin)
  • No Skin
Collapse
Code Project
  1. Home
  2. The Lounge
  3. Code reviews

Code reviews

Scheduled Pinned Locked Moved The Lounge
csharpc++designcode-reviewlounge
34 Posts 18 Posters 0 Views 1 Watching
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • N Nemanja Trifunovic

    Don't get me wrong - I like them in general. It's just that I spent one day implementing a feature and now three days and counting on the code review. At this point we aren't even discussing my changes, but the design of our unit-test system X| Let me check in already!

    utf8-cpp

    N Offline
    N Offline
    Nueman
    wrote on last edited by
    #2

    Nemanja Trifunovic wrote:

    Let me check in already!

    Not until we have world peace . . . ;P

    What me worry?

    1 Reply Last reply
    0
    • N Nemanja Trifunovic

      Don't get me wrong - I like them in general. It's just that I spent one day implementing a feature and now three days and counting on the code review. At this point we aren't even discussing my changes, but the design of our unit-test system X| Let me check in already!

      utf8-cpp

      T Offline
      T Offline
      thrakazog
      wrote on last edited by
      #3

      How's that saying go? "Meetings, because none of us is as dumb as all of us." I always felt code reviews went pretty much straight to that.

      Play my game Gravity: IOS[^], Android[^], Windows Phone 7[^]

      P 1 Reply Last reply
      0
      • N Nemanja Trifunovic

        Don't get me wrong - I like them in general. It's just that I spent one day implementing a feature and now three days and counting on the code review. At this point we aren't even discussing my changes, but the design of our unit-test system X| Let me check in already!

        utf8-cpp

        S Offline
        S Offline
        Septimus Hedgehog
        wrote on last edited by
        #4

        I worked with a spiteful piece of shyte at one job who got a competent contractor (who he didn't like but the rest of us did) dismissed because of his code review. C'mon, it was VB6 so what did he expect it to look like? The developer who did the review was a tosser. Maybe he was pee'ed off because four of us set up the companies first C# group and he wasn't in it. Done correctly, code reviews can be useful but not when it's used vindictively as that arsewipe did.

        "I do not have to forgive my enemies, I have had them all shot." — Ramón Maria Narváez (1800-68). "I don't need to shoot my enemies, I don't have any." - Me (2012).

        L J 2 Replies Last reply
        0
        • T thrakazog

          How's that saying go? "Meetings, because none of us is as dumb as all of us." I always felt code reviews went pretty much straight to that.

          Play my game Gravity: IOS[^], Android[^], Windows Phone 7[^]

          P Offline
          P Offline
          peterchen
          wrote on last edited by
          #5

          Statistically, code reviews - even informal ones - have one of the highest chance of catching bugs. Plus, they happen quite early in the process, further reducign the cost of those bugs. So yeah, maybe you, like, are, you know, doing it wrong.

          ORDER BY what user wants

          T R K W C 5 Replies Last reply
          0
          • N Nemanja Trifunovic

            Don't get me wrong - I like them in general. It's just that I spent one day implementing a feature and now three days and counting on the code review. At this point we aren't even discussing my changes, but the design of our unit-test system X| Let me check in already!

            utf8-cpp

            P Offline
            P Offline
            peterchen
            wrote on last edited by
            #6

            You should rather discuss the design of your code review process. It should not be used to talk about things that the reviewer wants to get of hisher chest. I'm sure someone at your place is aware of that.

            ORDER BY what user wants

            1 Reply Last reply
            0
            • P peterchen

              Statistically, code reviews - even informal ones - have one of the highest chance of catching bugs. Plus, they happen quite early in the process, further reducign the cost of those bugs. So yeah, maybe you, like, are, you know, doing it wrong.

              ORDER BY what user wants

              T Offline
              T Offline
              thrakazog
              wrote on last edited by
              #7

              peterchen wrote:

              So yeah, maybe you, like, are, you know, doing it wrong.

              That's a definite possibility. I've only worked at one place that tried code reviews. Those sessions always broke down into hour long discussions about how the white spacing didn't *look* right or a variable should have been named differently. Any bugs saved in those sessions were entirely imaginary. Your mileage may vary.

              Play my game Gravity: IOS[^], Android[^], Windows Phone 7[^]

              J P 2 Replies Last reply
              0
              • N Nemanja Trifunovic

                Don't get me wrong - I like them in general. It's just that I spent one day implementing a feature and now three days and counting on the code review. At this point we aren't even discussing my changes, but the design of our unit-test system X| Let me check in already!

                utf8-cpp

                B Offline
                B Offline
                BobJanova
                wrote on last edited by
                #8

                Code reviews before checking in seems like overkill to me. I like 'spot check' reviews where you look at each other's code now and then and make sure the style is okay. A continuous integration server and test discipline should ensure that you know your code is functionally correct, and style is vastly secondary to that and not worth holding up development as a matter of course for.

                J 1 Reply Last reply
                0
                • P peterchen

                  Statistically, code reviews - even informal ones - have one of the highest chance of catching bugs. Plus, they happen quite early in the process, further reducign the cost of those bugs. So yeah, maybe you, like, are, you know, doing it wrong.

                  ORDER BY what user wants

                  R Offline
                  R Offline
                  Ravi Bhavnani
                  wrote on last edited by
                  #9

                  :thumbsup: +5 /ravi

                  My new year resolution: 2048 x 1536 Home | Articles | My .NET bits | Freeware ravib(at)ravib(dot)com

                  1 Reply Last reply
                  0
                  • P peterchen

                    Statistically, code reviews - even informal ones - have one of the highest chance of catching bugs. Plus, they happen quite early in the process, further reducign the cost of those bugs. So yeah, maybe you, like, are, you know, doing it wrong.

                    ORDER BY what user wants

                    K Offline
                    K Offline
                    Kevin Marois
                    wrote on last edited by
                    #10

                    very code review I'v been in has been a complete waste of valuable development time. It usually turned into "Why did you do it this way???? My way is better...". Total waste

                    If it's not broken, fix it until it is

                    J P 2 Replies Last reply
                    0
                    • S Septimus Hedgehog

                      I worked with a spiteful piece of shyte at one job who got a competent contractor (who he didn't like but the rest of us did) dismissed because of his code review. C'mon, it was VB6 so what did he expect it to look like? The developer who did the review was a tosser. Maybe he was pee'ed off because four of us set up the companies first C# group and he wasn't in it. Done correctly, code reviews can be useful but not when it's used vindictively as that arsewipe did.

                      "I do not have to forgive my enemies, I have had them all shot." — Ramón Maria Narváez (1800-68). "I don't need to shoot my enemies, I don't have any." - Me (2012).

                      L Offline
                      L Offline
                      loctrice
                      wrote on last edited by
                      #11

                      PHS241 wrote:

                      Done correctly, code reviews can be useful but not when it's used vindictively as that arsewipe did.

                      :thumbsup:

                      If it moves, compile it

                      1 Reply Last reply
                      0
                      • T thrakazog

                        peterchen wrote:

                        So yeah, maybe you, like, are, you know, doing it wrong.

                        That's a definite possibility. I've only worked at one place that tried code reviews. Those sessions always broke down into hour long discussions about how the white spacing didn't *look* right or a variable should have been named differently. Any bugs saved in those sessions were entirely imaginary. Your mileage may vary.

                        Play my game Gravity: IOS[^], Android[^], Windows Phone 7[^]

                        J Offline
                        J Offline
                        jschell
                        wrote on last edited by
                        #12

                        thrakazog wrote:

                        Those sessions always broke down into hour long discussions about how the white spacing didn't *look* right or a variable should have been named differently. Any bugs saved in those sessions were entirely imaginary

                        Which represents a process failure at that location. Process failures can impact software development and even other types of processes at a company in many negative ways. That however is due to a failure in the way that the process is implemented and not a condemnation of what the process is attempting to achieve. The best way to produce effective processes is to make sure that managers and employees are specifically and directly responsible for the success of the process while providing avenues to change the process (but not eliminate it nor trivialize it.) If your bonus and your managers bonus is directly based on the success of your processes then both of you are going to be more invested in making it work.

                        1 Reply Last reply
                        0
                        • K Kevin Marois

                          very code review I'v been in has been a complete waste of valuable development time. It usually turned into "Why did you do it this way???? My way is better...". Total waste

                          If it's not broken, fix it until it is

                          J Offline
                          J Offline
                          jschell
                          wrote on last edited by
                          #13

                          Kevin Marois wrote:

                          Why did you do it this way???? My way is better

                          Code reviews can be a way of educating developers and also a way to insure that tribal knowledge increases. And that is often a good thing.

                          B 1 Reply Last reply
                          0
                          • S Septimus Hedgehog

                            I worked with a spiteful piece of shyte at one job who got a competent contractor (who he didn't like but the rest of us did) dismissed because of his code review. C'mon, it was VB6 so what did he expect it to look like? The developer who did the review was a tosser. Maybe he was pee'ed off because four of us set up the companies first C# group and he wasn't in it. Done correctly, code reviews can be useful but not when it's used vindictively as that arsewipe did.

                            "I do not have to forgive my enemies, I have had them all shot." — Ramón Maria Narváez (1800-68). "I don't need to shoot my enemies, I don't have any." - Me (2012).

                            J Offline
                            J Offline
                            jschell
                            wrote on last edited by
                            #14

                            PHS241 wrote:

                            ne correctly, code reviews can be useful but not when it's used vindictively as that arsewipe did.

                            However that is a management failure, not a process failure.

                            1 Reply Last reply
                            0
                            • B BobJanova

                              Code reviews before checking in seems like overkill to me. I like 'spot check' reviews where you look at each other's code now and then and make sure the style is okay. A continuous integration server and test discipline should ensure that you know your code is functionally correct, and style is vastly secondary to that and not worth holding up development as a matter of course for.

                              J Offline
                              J Offline
                              jschell
                              wrote on last edited by
                              #15

                              BobJanova wrote:

                              and test discipline should ensure that you know your code is functionally correct

                              And how do you know that your test discipline is correct if you do not review the tests?

                              B 1 Reply Last reply
                              0
                              • P peterchen

                                Statistically, code reviews - even informal ones - have one of the highest chance of catching bugs. Plus, they happen quite early in the process, further reducign the cost of those bugs. So yeah, maybe you, like, are, you know, doing it wrong.

                                ORDER BY what user wants

                                W Offline
                                W Offline
                                Wonde Tadesse
                                wrote on last edited by
                                #16

                                Yap. before it smells better to sanitize as early as possible.:thumbsup:

                                Wonde Tadesse

                                1 Reply Last reply
                                0
                                • N Nemanja Trifunovic

                                  Don't get me wrong - I like them in general. It's just that I spent one day implementing a feature and now three days and counting on the code review. At this point we aren't even discussing my changes, but the design of our unit-test system X| Let me check in already!

                                  utf8-cpp

                                  W Offline
                                  W Offline
                                  Wonde Tadesse
                                  wrote on last edited by
                                  #17

                                  Just wondering. Do you/team follow common coding standard/convention before starting implementations? I think this should be basic else it will be night mare to follow each dev code.

                                  Wonde Tadesse

                                  1 Reply Last reply
                                  0
                                  • T thrakazog

                                    peterchen wrote:

                                    So yeah, maybe you, like, are, you know, doing it wrong.

                                    That's a definite possibility. I've only worked at one place that tried code reviews. Those sessions always broke down into hour long discussions about how the white spacing didn't *look* right or a variable should have been named differently. Any bugs saved in those sessions were entirely imaginary. Your mileage may vary.

                                    Play my game Gravity: IOS[^], Android[^], Windows Phone 7[^]

                                    P Offline
                                    P Offline
                                    peterchen
                                    wrote on last edited by
                                    #18

                                    That's why there are coding standards. Stop edit wars of taste, and close a trivial discussion once and for all, see also bikeshedding[^]. A coding standard is the first thing to get before doing formal code reviews (When asking about what to prepare for a code review, I was surprised how highly this ranked. Now I know.) It doesn't even need to force one single specific style, it can allow different bracings, etc. A professional developers can stick to a coding style, even if it's not his favorite. If they can't, you need to agree on - or decree - an automatted formatting macro that is used for dispute resolution.


                                    Code Reviews are hard socially. We have better experience with informal ones ("Can we look together at...") - but they don't happen as often as I wanted to. I've read of others praising the advantages of formal ones.

                                    ORDER BY what user wants

                                    T 1 Reply Last reply
                                    0
                                    • K Kevin Marois

                                      very code review I'v been in has been a complete waste of valuable development time. It usually turned into "Why did you do it this way???? My way is better...". Total waste

                                      If it's not broken, fix it until it is

                                      P Offline
                                      P Offline
                                      peterchen
                                      wrote on last edited by
                                      #19

                                      There's a name for craftsmen that blame their tools, it's on the tip of my tongue. Code reviews are socially hard (which for some developers ranks slightly above NP-hard). As much as I despise "This is John Consultant, he will be with us for two days and tell you how to do your job", this is one situation where an external authority to drill individuals isn't the worst solution. It is problematic to get the right people behind it, and if it doesn't work by its own, you have to impose ridculously rigid rules. Not that you have to do code reviews, and there are likely shops who would never get it done. just don't throw out the coconut because the peel is so hard.

                                      ORDER BY what user wants

                                      1 Reply Last reply
                                      0
                                      • J jschell

                                        BobJanova wrote:

                                        and test discipline should ensure that you know your code is functionally correct

                                        And how do you know that your test discipline is correct if you do not review the tests?

                                        B Offline
                                        B Offline
                                        BobJanova
                                        wrote on last edited by
                                        #20

                                        You need to review test procedure, coverage and so on at some point. But the point I'm making is that it doesn't need to be reviewed before you check something in, and you don't need to review every single commit ... just a sample of what people are doing to check that they have the right habits.

                                        1 Reply Last reply
                                        0
                                        • N Nemanja Trifunovic

                                          Don't get me wrong - I like them in general. It's just that I spent one day implementing a feature and now three days and counting on the code review. At this point we aren't even discussing my changes, but the design of our unit-test system X| Let me check in already!

                                          utf8-cpp

                                          3 Offline
                                          3 Offline
                                          3n1g
                                          wrote on last edited by
                                          #21

                                          Never experienced that, as I work on a small company, and I am currently the single developer of the 1 big project and 1 smaller one (getting bigger though). Sometimes I feel I could use some feedback, as I never really was "under the wing" of a more experienced developer, and kind of "learn" things on the fly without any critique by anyone else than me. But yes, 1 day coding and 3 days circle-jerking around it sounds a bit too much.

                                          1 Reply Last reply
                                          0
                                          Reply
                                          • Reply as topic
                                          Log in to reply
                                          • Oldest to Newest
                                          • Newest to Oldest
                                          • Most Votes


                                          • Login

                                          • Don't have an account? Register

                                          • Login or register to search.
                                          • First post
                                            Last post
                                          0
                                          • Categories
                                          • Recent
                                          • Tags
                                          • Popular
                                          • World
                                          • Users
                                          • Groups