October 3, 2015 at 12:50 pm
Comments posted to this topic are about the item Write Cleaner T-SQL Using Version Control
October 5, 2015 at 12:27 am
There are other potential benefits. Here are some that spring to mind:
1) Having a local copy of all database files means that you can search your database without hitting the server. Not only this, but the tools which are available to do this search have more power (eg, a Regex search) than those in SQL Server (even though SQL Search is great for this too).
2) It's possible to do bulk database refactoring by building tools which start by modifying the files in your local repo. Then use your VCS to do a schema compare of what's in the repo vs what's in the DB and, if all looks well, apply the changes.
If all does not look well, roll back the changes, modify the file-update tool and try again. You have not hit the database yet, so no one else has been impacted.
I've used this method to replace dbname..object with dbname.dbo.object, for example (lots of procs affected).
3) Running hand-in-hand with DB version control are VS database projects. If you ensure that your DB can be 'built' successfully, with no errors or warnings, you can be assured that your deployed database is in a consistent state.
The absence of evidence is not evidence of absence
- Martin Rees
The absence of consumable DDL, sample data and desired results is, however, evidence of the absence of my response
- Phil Parkin
October 5, 2015 at 5:50 am
[font="Arial Black"]From the Article:[/font]
So if we take out the code that has been commented out by not being reachable we end up with:
create procedure usp_DoSomething (@PARAMETER int)as
select top 1
a, b, c, d, e
from table
where p = @PARAMETER;This is much more succinct and easier to read and understand.
Version control is nice and a great idea but if you end up with comment-less code like the above, then you're using it for the wrong thing.
Yes, I agree that you don't need to include the name of the store procedure in a comment header because it's already there. I also agree that you don't need to list the revision history in the code because it's in the version control system and I agree that you can and should remove commented out code provided that you list the actual reason why the code was removed in VCS. But the code gives no clue as to what it's for or how to use it. That means that you HAVE to go to another place to find out what this code is about.
Then there's the joy that comes when someone decides that a different VCS is supposed to be used and the data in the old VCS either can't be migrated or someone simply forgets to do it and all that information is lost.
Not having a properly designed and useful production header is the worst idea possible whether VCS is at play or not. Stop making excuses for not including proper documentation in code.
As a bit of a sidebar, if all of your stored procedure names start with "USP_" and the whole stored procedure is written in a single case (lower case in the example above), then you have some other additional serious problems that VCS isn't going to help you with.
--Jeff Moden
Change is inevitable... Change for the better is not.
October 5, 2015 at 6:00 am
Jeff Moden (10/5/2015)
[font="Arial Black"]From the Article:[/font]
So if we take out the code that has been commented out by not being reachable we end up with:
create procedure usp_DoSomething (@PARAMETER int)as
select top 1
a, b, c, d, e
from table
where p = @PARAMETER;This is much more succinct and easier to read and understand.
Version control is nice and a great idea but if you end up with comment-less code like the above, then you're using it for the wrong thing.
Yes, I agree that you don't need to include the name of the store procedure in a comment header because it's already there. I also agree that you don't need to list the revision history in the code because it's in the version control system and I agree that you can and should remove commented out code provided that you list the actual reason why the code was removed. But the code gives no clue as to what it's for or how to use it. That means that you HAVE to go to another place to find out what this code is about.
Then there's the joy that comes when someone decides that a different VCS is supposed to be used and the data in the old VCS either can't be migrated or someone simply forgets to do it and all that information is lost.
Not having a properly designed and useful production header is the worst idea possible whether VCS is at play or not. Stop making excuses for not including proper documentation in code.
Hi Jeff - no excuses here, I write code and document it where required - I actually spend more time documenting in a more useful way such as writing tests that actually show how the code should be used and prove that it actually works.
I also ensure that code is clear and well named.
If neither of these approaches are good enough then I document it.
Code commenting should explain the why where it is not obvious and until someone can guarantee that people never make mistakes in their comment headers and so are always accurate I won't trust them - this is from experience in a number of different environments.
ed
October 5, 2015 at 7:16 am
I'm with Jeff on the header saying what the SQL is supposed to do. When debugging if a header says what it's supposed to do and the code does something else I know the mistake is in the code. If the header says what it's supposed to do and it does that then I know we have a design error and I have to broaden my thinking. It's very helpful in cases where the code is complex. Having test procedures is all very helpful too but if I'm handed a project with SQL how do I even know those test cases exist and where they are, unless you know a way to attach them to the statement that I haven't found.
October 5, 2015 at 7:27 am
Taking out the name and parameters makes sense. Those are already in the code.
I don't agree with taking out the descriptive parts because stored procedures can and will be pulled from the database. Without that information it's useless to have a quick look at a stored procedure to see what's been going on.
Something we add to our template is a section that shows how the sproc is called including the EXEC. It's a handy way to select and run it as well as do a copy-paste as needed.
Version control is great but think about how people dig into the database and work outside of version control even when they should know better.
October 5, 2015 at 7:38 am
JustMarie said:
Something we add to our template is a section that shows how the sproc is called including the EXEC. It's a handy way to select and run it as well as do a copy-paste as needed.
Great call, I use that all the time as well. Another one I use is to leave the interim select statements that I use for testing between CTE segments in the code but commented out. They make it really fast to find a problem with a very complex set of CTE's and don't, in my opinion, muck up the code at all.
October 5, 2015 at 7:44 am
JustMarie (10/5/2015)
Taking out the name and parameters makes sense. Those are already in the code.I don't agree with taking out the descriptive parts because stored procedures can and will be pulled from the database. Without that information it's useless to have a quick look at a stored procedure to see what's been going on.
Something we add to our template is a section that shows how the sproc is called including the EXEC. It's a handy way to select and run it as well as do a copy-paste as needed.
Version control is great but think about how people dig into the database and work outside of version control even when they should know better.
I think that people shouldn't be messing with the database, what we need is a proper development process that goes through testing and review rather than just changing stuff and seeing what happens - imagine if you deployed a cpp app and the ops team changed the assembly code to fix issues - that would be seen as mostly ridiculous :).
Just because you can change the database code pretty easily really doesn't mean that you should 🙂
October 5, 2015 at 9:00 am
billp 37934 (10/5/2015)
I'm with Jeff on the header saying what the SQL is supposed to do. When debugging if a header says what it's supposed to do and the code does something else I know the mistake is in the code.
I'd disagree here. It could be the case, but it could also be the requirements evolved, the code gets changed, but the header comment doesn't.
I think that this is where a VCS is handy. The habits of a developer evolve to enter the comment when checking in, since that's a prompted requirement. I see developers forgetting to update code comments, which is a problem in and of itself, but because this isn't required in a work flow, it's an issue.
October 5, 2015 at 9:05 am
I didn't mean to imply instead of version control, just in addition to version control.
October 5, 2015 at 9:09 am
I fully support VCS and in my development environment the db portion is the most lacking in this area. 🙂 I appreciate the different perspectives the author has provided in support of version control for the database code. A fully integrated environment that includes version control provides many many advantages and techniques that can be used on a day to day basis.
So will the next article be a comparison of some VCS's and their benefits to include which development environments they can easily be incorporated into? I use SSMS and find it difficult to incorporate VCS (Subversion) and have it completely integrated. Plugins for Eclipse seem to be kludgy and don't allow all the benefits of working in SSMS. Would really appreciate more on this from those with more experience than I have.
Thanks!
October 5, 2015 at 9:13 am
I agree. I like the comments in the stored procedure. I work in a company where each department and group within the department have their own source control - most of which I do not have access to. I also find that people who can explain in a simple sentence what they are trying to do write better code.
October 5, 2015 at 9:15 am
nfink (10/5/2015)
I fully support VCS and in my development environment the db portion is the most lacking in this area. 🙂 I appreciate the different perspectives the author has provided in support of version control for the database code. A fully integrated environment that includes version control provides many many advantages and techniques that can be used on a day to day basis.So will the next article be a comparison of some VCS's and their benefits to include which development environments they can easily be incorporated into? I use SSMS and find it difficult to incorporate VCS (Subversion) and have it completely integrated. Plugins for Eclipse seem to be kludgy and don't allow all the benefits of working in SSMS. Would really appreciate more on this from those with more experience than I have.
Thanks!
Hi nflink,
Have a look at:
You can't go far wrong choosing ssdt, you get rename refactoring, compile time validation and automatic / compare & merge type deployments (all for free with visual studio community / express)
Ed
October 5, 2015 at 10:06 am
Thanks for posting the article. It's important to look for ways to reduce comments down to only that content that is useful. I agree on removing the sproc name and parameters from the header - just one more thing to have to maintain and provides no real value. With respect to history documentation, we have gone through 2 source control migrations and have had limited success retaining history so I'd add my voice to the chorus about code history within the procedure being of value. I'd also argue that the context of looking at the comments in the object itself is better than browsing through source control comments which may have been generalized for a set of objects changed in the same check-in.
October 5, 2015 at 10:09 am
Oh PERFECT! Thank-you very much!
Viewing 15 posts - 1 through 15 (of 26 total)
You must be logged in to reply to this topic. Login to reply