April 24, 2015 at 7:34 am
No we don't do code reviews in a formal sense. These days the excuse is 'not enough time' but I think that's shortsighted. I have learned the most about programming and problem-solving by listening to how other people do it. None of us can learn it all on our own (at least in one lifetime) and all of us can all profit from sharing our work and getting that feedback. I miss it.
Sigerson
"No pressure, no diamonds." - Thomas Carlyle
April 24, 2015 at 7:49 am
Good Editorial.
We peer review all production changes including DBA scripts as part of our Release Management process. This is part of our ITIL compliance.
April 24, 2015 at 7:50 am
When I worked in the video game industry, we surely did code reviews all the time.
Now that I work in a different industry specifically with SQL Development, we don't only because I'm the only developer.
However, we were just acquired by a bigger company with a proper dev team and pipeline where they do code reviews.
In my opinion, especially in positions like mine where I write massive queries that aggregate massive data sets, code reviews are important.
April 24, 2015 at 10:06 am
I've seen code reviews required in some shops, little or none in others.
An old adage applies: "An ounce of prevention is worth a pound of cure." Sometimes 10 pounds, if the damage from a mistake is bad enough.
Another adage applies: "Pay me now, or pay me later."
But code reviews are a 2 edged sword:
> in small companies, the time and manpower to perform 100% code reviews may be more than they can afford.
> in large companies, the time and manpower to perform 100% code reviews usually exists, and it is a best practice, if they can afford it.
Bottom line: Code reviews are considered a best practice for good reasons. Do them if, and to the degree, you can afford them.
One other factor is a consideration: The expertise of code implementers.
> If implementers are highly effective, code reviews are less valuable.
> If implementers are less skilled, the time and money spent on code reviews are extremely valuable.
My personal execution: I almost always ask for a code review from a co-worker, even if my employer does not require them. I am a rigorous implementer and tester, who performs 100%, line-by-line testing, including all data limit conditions. So code reviews are not so much time and money well spent. I do them anyway. They are a best practice.
One last thing: My last job was with a very large company. Their CMS (Change Management System), which also included code reviews, is probably going to put them out of business in this calendar year. Their process, as defined, was so large, so complex, with so many steps, so many management signoffs, as to be impractical. For various reasons, their need to fix production problems is increasing faster than their process of resolution can fix them.
Code review processes need to be rational and practical, to further business objectives, rather than be a hindrance to meeting objectives.
April 24, 2015 at 10:55 am
Good to see some of you doing code reviews. Not a lot of response, so I suspect many aren't.
Those of you doing testing as well, we'd love to hear more, or get articles on the ways in which you test specific code items. More examples are always helpful to guide others.
April 24, 2015 at 11:05 am
below86 (4/24/2015)
"Code reviews? We don't need no stinkin' code reviews."
:crazy:
April 24, 2015 at 12:48 pm
We do code reviews on everything unless it's trivial (like a label change) primarily for adherence to coding standards and use of the right patterns in the right situations, but also for code efficiency and second opinions on general approach to problems. I think it's a very valuable venue for learning; there's quite a bit of "instead of x, you could do y", because it would be more efficient, or compact, or modular, or whatever.
We use a tool called ReviewBoard (which I believe is open source) which I like because it makes the reviews asynchronous (reviewers can review at their leisure with no need to schedule times) and helps track the issues and changes nicely.
April 24, 2015 at 1:06 pm
Gail Wanabee (4/24/2015)
I've seen code reviews required in some shops, little or none in others.My personal execution: I almost always ask for a code review from a co-worker, even if my employer does not require them. I am a rigorous implementer and tester, who performs 100%, line-by-line testing, including all data limit conditions. So code reviews are not so much time and money well spent. I do them anyway. They are a best practice.
In my group they're not required. I too try to be rigorous and test all execution paths. I'll still ask someone to test for me to check my assumptions. This my be more of a QA issue but there have been times when I've done everything right within what I was doing; only to learn it was wrong from a business perspective. A tester familiar with the business rules can point that out.
Ken
April 24, 2015 at 3:23 pm
ken.trock (4/24/2015)
Gail Wanabee (4/24/2015)
I've seen code reviews required in some shops, little or none in others.My personal execution: I almost always ask for a code review from a co-worker, even if my employer does not require them. I am a rigorous implementer and tester, who performs 100%, line-by-line testing, including all data limit conditions. So code reviews are not so much time and money well spent. I do them anyway. They are a best practice.
In my group they're not required. I too try to be rigorous and test all execution paths. I'll still ask someone to test for me to check my assumptions. This my be more of a QA issue but there have been times when I've done everything right within what I was doing; only to learn it was wrong from a business perspective. A tester familiar with the business rules can point that out.
Ken
So this is good, but how do you perform 100%, line by line testing? Or testing of all execution paths? I'd really like to know, and if you have samples, this would make a great article or blog post.
April 24, 2015 at 8:18 pm
Steve Jones - SSC Editor (4/24/2015)
Good to see some of you doing code reviews. Not a lot of response, so I suspect many aren't.Those of you doing testing as well, we'd love to hear more, or get articles on the ways in which you test specific code items. More examples are always helpful to guide others.
When it comes to testing, I unit test everything I do. SQL is very good for unit testing piece by piece. Like most, I start by writing as much as I can in my rough draft on the request. Then I go back and unit test each section of my code to ensure that every unit is working as expected. Then I start optimizing my code based on those unit tests to ensure I'm at least following some best practices learned from my own resources like books, guides and even the great people of this forum.
I know that I'm not the best and that I'm not perfect. I'm not trying to be perfect. I'm trying to get stuff done and stuff done that's not going to through wrenches into the engine through each unit I test.
The next set of tests I sometimes do include is trying to measure the impact of my final product on the server. I want to ensure that my unit testing holistically is paying off and If I need to continue optimizing my query some more. Being I work with large data sets that include big reads of millions, sometimes hundreds of millions of records, I can really bring the pain to the server if I'm not careful, especially with attribution reporting.
Then lastly, being I mostly do a lot of reporting specific queries, I hand off the final product to a co-worker who will check and ensure the business logic is what they want. The last thing I want to do is flub up the output where we I'm outputting incorrect numbers that cause the team to make incorrect assumptions about the data. This could cost me my job or cost us business if I'm not 120% correct.
So, yeah, the thought of no testing would kill me. I would likely walk away from a project that did not take testing seriously enough to sanity check my work.
April 25, 2015 at 8:25 am
I wish we did code reviews. I sometimes have others sit down with me and review my code, but usually when it's something new or when they might have to work with it. For the normal day-to-day, no. Sigh. :ermm:
It's about time and workload. I don't mean that as an excuse and I wish it were not the case.
April 25, 2015 at 9:19 am
xsevensinzx (4/24/2015)
I know that I'm not the best and that I'm not perfect. I'm not trying to be perfect. I'm trying to get stuff done and stuff done that's not going to through wrenches into the engine through each unit I test.
That's the perfect attitude for testing. If you'd like to share specific examples in an article, we'd love to know how you pick up stored proc x (or code y) and write test(s).
April 26, 2015 at 12:16 am
1. First, the development team will do unit testing.
2. Then, me the DBA will review the code before launching to UAT environment & will do the neccessary changes to SP`s & fn`s if neccessary.
3. The QA/testing team will test the functionality of the implemented changes in the application.
4. Will get the change code reviewed & approved by the App. Dev. manager before hitting live production servers.
Thanks & Best Regards,
Hany Helmy
SQL Server Database Consultant
April 26, 2015 at 5:36 am
Unfortunately it seems that the database world lags a bit behind the code world in terms of tooling and approaches.
Products like SonarQube help in the code world as they allow a certain amount of self-policing. SonarQube does support PL-SQL but I'm not sure it supports T-SQL properly. I have heard that it expects databases to be installed as case sensitive which opens up a can of worms.
Beyond the "Yes we have code reviews" I would ask how you approach code reviews. I deliberately picked 2 nit picking curmudgeons to review my code because they could be guaranteed to find something wrong. If I could satisfy them then the code was a good as it could be. Mind you each of the other 2 curmudgeons used to use me for code reviews so what does that say about me:doze:
Manual code reviews are fine up to a point but having watched the code development world I think we need to make automated code reviews more of a priority.
Grant Fitchley reviewed SQLCop [/url]and there are others out there.
The beauty of mechanical reviews is that they are impartial and a yard stick against which you can measure yourself. Personalities don't come into play. There is no falling out between colleagues because of differences of opinion.
The other key advantage of products like SonarQube is that they provide useful metrics and also present metrics that will appease meddling managers. The worst thing you can do is introduce a process that generates a heavy weight measuring process that need to be "managed".
April 27, 2015 at 6:56 am
Actual code reviews by other programmers is done when the writer asks for a second opinion. So it doesn't happen very often, unless that programmer is newly hired. We do have an extensive set of testing environments that newly written code is put into and through. This helps catch most errors but some slip through that code review may have caught.
Viewing 15 posts - 16 through 30 (of 45 total)
You must be logged in to reply to this topic. Login to reply