61–67 of 67 posts
Earlier quoted context omitted.
Are code reviews by non-programmers actually common? How can that possibly work?
I've seen 'code reviews' that were in fact functionality checks. So it's more like a 'check if this code works'. And I can tell you: a web form capable of SQL-injections is still 'working code'. So it gets a positive 'code review'.
Earlier quoted context omitted.
Code review is about reviewing the code. That's literally what the name says. CI runs the tests, QA test the final behavior, code review checks the code.
That's nice in theory, but in practice another dev really should be running the code and doing some manual testing. Having full knowledge of the innards allows a competent dev to foresee potential trouble spots in the architecture and hit some edge cases to make sure they don't break. CI may run the tests, but that assumes your tests have captured every possible edge case. It may be the intention of tests to be compr…
Why? If they are reviewing the code including the unit tests, shouldn't they instead be suggesting any missing unit tests, which then become permanent and reusable rather than "doing some manual testing"?
> CI may run the tests, but that assumes your tests have captured every possible edge case. It may be the intention of tests to be comprehensive and cover everything under the sun, but that is rarely the case in reality.
Insofar as the other dev doing code review can address this with their own testing, isn't it better for this to be done-once and preserved by adding automated tests rather than done-and-lost by doing manual tests?
When talking about code reviews I am constantly reminded of the following 1. How hard it is to grok code that I've never seen before 2. How hard it is to grok my own code from 6 months ago Sometimes it may take me a day or more to fully get my head around a new piece of code I am working with. I could not give a thorough AND fair review of another person's code without internalizing it to a satisfactory degree. There…
> Sometimes it may take me a day or more to fully get > my head around a new piece of code I am working with. Are you trying to perform a design review, or a code review? Code reviews (in my mind, at least) concern themselves with the low-level mechanics of the code ("Handle this exception properly","there's a library method for this logic","follow the team's coding standards", etc.) whereas design reviews deal with…
When I review code I review all of it, not just the easy parts.
If I was going to do a 1/2 ass job of it I wouldn't do it in the first place.
Earlier quoted context omitted.
See six steps to success section of my write-up of Dr. Holzmann's talk on how JPL builds reliable software. http://www.verticalsysadmin.com/making_robust_software/ The clever idea is to make your coding standard mechanically verifiable.
Thanks, nice list, summarize a good part of the best practices in a short form. Will quote.
I need to write a shorter version, to make the data more broadly accessible.
Proper code review provides much more benefit than cost and is indispensible for quality software. Improper code review is a waste of time (or worse). Proper code review is done: - by another programmer - by someone with some knowledge of the application - by someone with some knowledge of the environment - against some code standard - against standard requirements (APIs, database, etc.) - with a checklist - uniforml…
I also include junior developers who don't know much about the application or environment in code reviews. It's a good way for them to learn the code base and see what type of defects the senior developers point out.
Earlier quoted context omitted.
My comments are more along the lines of alert("GOT HERE 1"); alert("GOT HERE 2"); alert("GOT HERE 3"); Not very helpful if someone else sees it, but also not profanity.
They can be quite irritating for others though, especially if they happen during a loop so that the user will have to click past 40 popups to access something.
Earlier quoted context omitted.
> Sometimes it may take me a day or more to fully get > my head around a new piece of code I am working with. Are you trying to perform a design review, or a code review? Code reviews (in my mind, at least) concern themselves with the low-level mechanics of the code ("Handle this exception properly","there's a library method for this logic","follow the team's coding standards", etc.) whereas design reviews deal with…
My time is more valuable than being a pre-compile/pre-deploy sanity check or human lint. When I review code I review all of it, not just the easy parts. If I was going to do a 1/2 ass job of it I wouldn't do it in the first place.
> When I review code I review all of it...
Design reviews can be done fairly early in the process before the code is complete. If design changes are needed, there is still plenty of time for them.
Code reviews can't really be done until most of the code is written, but (as you point out) the kinds of problems they find are much easier to fix, so require less time.