I don't generally stand by the idea of peer-reviewing all code, not at least until a product is passed from it's original creation pass, that boot strapping has to be all done and over and a certain amount of confidence in the product has to stand before I believe every change needs to be peer-reviewed (certainly group code review internally as you produce that first sprint to a working product - and remember always try to push sprints to working builds, never break things - but don't ask everyone to peer-review every change when there are literally hundreds happening an hour as a system comes online). Anyway, back to my point...
I assigned my work for review, we're using git, so the pushes upstream get logged and anyone in the team can pick to peer review, the ticket system I'm putting together myself shows whether a change was peer-reviewed or not, but does not make them mandatory.
New developers joining my team however have to spend an amount of time loading the code into their wet-ware via eye-ball MK1, so it's worth them subscribing to the push feed and doing peer-reviews.
And I pride myself on picking the better developers for my teams, it saves me too much time getting them up to speed.... Soooo.... what's my point? Well..
I just used a Lambda in some C++. The chap peer reviewing it rejected the code. As he thought that it was gibberish….
auto l_Thread = std::shared_ptr<std::thread>(
new std::thread([]()
{
std::this_thread::sleep_for(
std::chrono::milliseconds(500));
InitButtonsAgain();
});
His comments told me more about his skills than I think he wanted them to...
"Auto is an unknown type at compile time (is it even a type?)"
"Excessive use of std:: use using namespace std; at the time
"[]() is invalid syntax"
"The function braced on like 3 to the last line of the change has no name"
My unfortunate, and impatient, reply was "Update your skills to C++14".
No comments:
Post a Comment