Given that the reviewer is not part of the project, but was assigned to review because he has done some coding for the application being updated/enhanced.
Is it the reviewer's job to ensure that the requirement is complete and correct? Or is this the QA and business tester's responsibility?
If the developer and the reviewer misses a requirement which is discovered in UAT, is the reviewer also at fault (probably to a lesser degree)?
It obviously depends on your code review process but personally I'd say the functional completeness of the requirement is more within the remit of the QA / Test team than the code reviewer.
That's not to say that a code reviewer shouldn't be able to pick up on problems with the functionality where they find them (assuming they have the knowledge to identify them), just that it's not their primary aim.
I'd generally see code review as looking to uncover coding errors and inefficiencies, vulnerabilities, memory leaks, that is not checking that what has been implemented is correct against the specification, but that it is "correct" within itself.
If you do want to extend it beyond this to include more of a functional review, you need to accept that that's going to have an impact on the time you need commit to it. If you want the reviewer to be fully familiar with the functional requirements and to be checking them as well, then you obviously need to allow a lot more time for reading and understanding the requirements and checking the code against them.
But it shouldn't be seen as a substitute for proper QA and testing, at most this would be an additional layer.
Broadly speaking:
If the reviewer spots any problem he should definitely mention them (especially if he is the SME).
But when I receive code that needs to be reviewed I am concentrating at the function level. I see my primary job as reviewer is to make sure the code is readable and maintainable in the future (if I spot bugs I will point them out but I am not going to write unit tests (though if I spot missing unit tests I will point that out)).
Maintainability covers a lot of ground. Some of the things I look for are: style (though anything I point out in style are suggestions and I don't require them to be fixed). Obvious errors or coding styles that can cause problems or that will make the code hard to extend. Algorithms that are very efficient (and there is a simple know better one). Anything that is not trivial self documenting that does not have a description of what the goal of the code is.
It is the job of tests and QA to make sure it does a sufficient job.
What you look for in a code review depends what you hope to accomplish it and what type of code review you are conducting. Some code reviews focus on security and looking for vulnerabilities, others look at style and conformance to coding guidelines, while others are just a sanity check for the code. Typically, the reviewer is told (either per guidelines or in the form of a checklist) what kinds of problems the code review is looking for, so their attention can be focused on critical sections.
If you are concerned about high quality, you probably want to introduce reviews at each phase. Requirements reviews can be used to ensure completeness and understandability of requirements. Design reviews are used to ensure that all requirements are addressed by the architecture and design of the system. Code reviews can be used to ensure that the code matches the design and doesn't contain errors before testing, as well as to review the strength of test cases.
When it comes to ensuring that requirements are properly implemented, it's everyone's responsibility, from the requirements engineer to document it to the architect and developers to understand it, design for it, and implement it to the testers who write test cases and test procedures. If anyone identifies an ambiguity, conflicting requirements, or some aspect of the system that doesn't meet a specified requirement, that individual needs to document it and somehow report it (usually by reporting a defect in your bug tracker).
Is it the reviewer's job to ensure that the requirement is complete and correct? Or is this the QA and business tester's responsibility?
It is the QA (Software quality assurance) responsibility to ensure that the requirement is complete and correct. Anyone assigned to do that wears tester's hat no matter how large is his developer's badge.
(source: dilbert.com)
It all comes down to what YOU want him to do. But generally speaking, I think of code review as a very specific function. He is there to check for the code. Make sure the code is up to standards and optimally written. If you make your code reviewer do the same thing your Q&A people will do, then you'll be doing double the work for double the time, and the same results. Which is often unnecessary.
This becomes more critical when you have several projects. You can't have 1 reviewer spend too much time on any one project, because Q&A is waiting to test it and you delay the entire process.
Which is why functions need to be clearly defined, as to not duplicate work.
I'll agree with Jon Hopkins answer that a code review should be mostly about the code, not the requirements.
However, if somehow the basic architecture, design or algorithms of the code under review simply cannot meet the core requirements or the spec, or are highly non-optimal for such, one might need to point this out at the beginning of the review so that much time isn't wasted going down a dead-end path.
