Building Helpful Commit Messages

  • This was removed by the editor as SPAM

  • Data_Cat, I agree with peter.row about going to Git over TFS.  Since Microsoft now owns GitHub, I expect they will move in that direction and eventually sunset TFS.

  • larry.blake wrote:

    Data_Cat, I agree with peter.row about going to Git over TFS.  Since Microsoft now owns GitHub, I expect they will move in that direction and eventually sunset TFS.

    Now that Microsoft has AzureDevops, which allows you to have a Git repo then you can still go with MS if you are an MS shop. I know they bought Github but AzureDevops adds continuous integration build functionality as well as the option to use it for writing up tasks, doing code reviews, etc...

    I've just the other day done a test migration of our on premise SVN repo for my project to a Git repo on AzureDevops. It was a little hellish to migrate the commit history and tags the way I wanted but I got there in the end with the help of colleagues and StackOverflow.

  • I've never put anything in the Commit Message (never is too strong, but certainly not individual notes on each COMMIT such a Ticket No related to the change I made). What am I missing?

    I can't think of an instance where I needed to find the code changes related to a particular work ticket ...

    We use a separate file for each Sproc, View, Trigger, Function, etc. and put them/changes in SVN

    My starting point is that I am not expecting to have to retrieve content from SVN ... if I do have to then "something bad has happened"

    In that situation I am mostly looking at the CHECKIN dates for that source file to figure out how far back I am likely to have to DIFF to review the changes made in order to figure out at what point the problem arose.

    I also use SVN for Peer Review of code but I haven't found a good way to do that because what I really want is "What is more recently checked in than my local copy, and DIFF those files to review / approve changes". Perhaps that is possible with SVN but I haven't found a way - maybe that is better served by GIT?

    The method I used for Doe Review / Compliance, which is pretty rubbish, is:

    • Copy all my local code to Safe Folder
    • "SVN UPDATE" my local code (i.e. overwrite with other people's more recent changes)
    • Copy back my Safe Copy to Overwrite ALL the other users' code
    • Now DIFF that and "adopt" all the changes that I approve 🙂 and then CHECKIN

      Basically I am adopting the code changes of my colleagues and at the same time fixing any layout / comment issues.

      I also (as a separate process) document to them why I changed their code as part of our ongoing "compliance"

     

  • Kristen-173977 wrote:

    I've never put anything in the Commit Message (never is too strong, but certainly not individual notes on each COMMIT such a Ticket No related to the change I made). What am I missing?

    I can't think of an instance where I needed to find the code changes related to a particular work ticket ...

    We use a separate file for each Sproc, View, Trigger, Function, etc. and put them/changes in SVN

    My starting point is that I am not expecting to have to retrieve content from SVN ... if I do have to then "something bad has happened"

    In that situation I am mostly looking at the CHECKIN dates for that source file to figure out how far back I am likely to have to DIFF to review the changes made in order to figure out at what point the problem arose.

    I also use SVN for Peer Review of code but I haven't found a good way to do that because what I really want is "What is more recently checked in than my local copy, and DIFF those files to review / approve changes". Perhaps that is possible with SVN but I haven't found a way - maybe that is better served by GIT?

    The method I used for Doe Review / Compliance, which is pretty rubbish, is:

    • Copy all my local code to Safe Folder
    • "SVN UPDATE" my local code (i.e. overwrite with other people's more recent changes)
    • Copy back my Safe Copy to Overwrite ALL the other users' code
    • Now DIFF that and "adopt" all the changes that I approve 🙂 and then CHECKIN Basically I am adopting the code changes of my colleagues and at the same time fixing any layout / comment issues. I also (as a separate process) document to them why I changed their code as part of our ongoing "compliance"

    The reason to put the work ID/ticket ID in the commit message is so that if, for example, there is a problem, say in an stored proc file, you can use the ID from the commit message to look up the task and give context as to why this change was made. Or to see what other files where changed at the same time. Without that you could "fix" something whilst break the work done by the task that changed it.

    We use JIRA for tasks and use the Crucible plug in. What this does is on the task there is a "Development" section. When commits are done with the task ID in, e.g. XX-1234 then Crucible will automatically pick that up and in the dev section on the task say that there are X commits. You can then make a Crucible review out of 1 or more of those commits. This presents you with a web front end that shows the changed files from those commits in tree view. As you select each file it shows you a diff' on the right. The code reviewer can then highlight individual lines or multiple lines and add a comment, e.g. "select * is bad practice in a query returning data, change it to select column names". You can mark the comment as "needs resolution". After the developer has made the changes and, optional, replied to the comments on the review, the new commits can be added to the review and you can then mark the comment as resolved.

    Using something like JIRA/Crucible (Azure Devops has similar functionality as well as repo hosting) is superior to just diff'ing the changes in the commits in SVN in every single way. Trying to explain that in line X in file Y in folder Z has [problem] in comments, especially for larger tasks, is the definition of torture. And if the task needs to be reopened more than once then it also leads to review issues being missed or skipped.

    You are right that using Git instead of SVN won't have any effect on your code review process.

    FYI - we literally just got a test migration of our projects SVN repo to Git hosted in AzureDevOps done the other day. It took me a while to figure it out and get it right. It takes about 26 hours to migrate nearly 20K commits, because I want to keep the SVN commit history but it worked in the end. If you don't care about your commit history or tags then you can simply just take a snapshot of your latest codebase and be done inside an hour or 2.

    • This reply was modified 3 years, 10 months ago by  peter.row. Reason: grammar
    • This reply was modified 3 years, 10 months ago by  peter.row.
  • Kristen-173977 wrote:

    I've never put anything in the Commit Message (never is too strong, but certainly not individual notes on each COMMIT such a Ticket No related to the change I made). What am I missing?

    I can't think of an instance where I needed to find the code changes related to a particular work ticket ...

    We use a separate file for each Sproc, View, Trigger, Function, etc. and put them/changes in SVN

    My starting point is that I am not expecting to have to retrieve content from SVN ... if I do have to then "something bad has happened"

    In that situation I am mostly looking at the CHECKIN dates for that source file to figure out how far back I am likely to have to DIFF to review the changes made in order to figure out at what point the problem arose.

    I also use SVN for Peer Review of code but I haven't found a good way to do that because what I really want is "What is more recently checked in than my local copy, and DIFF those files to review / approve changes". Perhaps that is possible with SVN but I haven't found a way - maybe that is better served by GIT?

    The method I used for Doe Review / Compliance, which is pretty rubbish, is:

    • Copy all my local code to Safe Folder
    • "SVN UPDATE" my local code (i.e. overwrite with other people's more recent changes)
    • Copy back my Safe Copy to Overwrite ALL the other users' code
    • Now DIFF that and "adopt" all the changes that I approve 🙂 and then CHECKIN Basically I am adopting the code changes of my colleagues and at the same time fixing any layout / comment issues. I also (as a separate process) document to them why I changed their code as part of our ongoing "compliance"

    In addition to my previous response. You would be better served if your developers created feature branches based on trunk, when done you code review and test from that feature branch. Once testing passes you merge that feature branch into trunk. The implementing developer should be the one who does the merge. If you are not happy with code in the feature branch:

    • You re-open their task stating the issues
    • They implement changes
    • You code review again

    The above repeats until you are happy. This reinforces the project practices, if you change things on their behalf they won't really learn from their mistakes because they aren't having to fix them.

  • peter.row wrote:

    The reason to put the work ID/ticket ID in the commit message is so that if, for example, there is a problem, say in an stored proc file, you can use the ID from the commit message to look up the task and give context as to why this change was made. Or to see what other files where changed at the same time. Without that you could "fix" something whilst break the work done by the task that changed it.

    Thanks all that you have said is interesting, thanks for taking the time to explain. On reflection I think that (for me) the SVN Commit Message is not the place I would drive that from. I have that (ticket) information in (a comment) the source code (which is probably only any good for "For Info"), but more usefully for me I have the means to see (for a given ticket) what source code was changed. For us it would be very rare that a single checkin matched a single work ticket. Cross reference between tickets causes us to try to mop up any related jobs at the same time, and I wouldn't bother to checkin more than once a day, often less frequently. We are a small shop though ... if I was trying to land on Mars I reckon I'd need something far more sophisticated ...

    This is a bit of a tangent, hopefully that's OK :

    My Time Sheet record requires me to enter the ID of associated Work Ticket. So I know who/when/how long that Ticket was worked on.

    Every individual script file we have (Sproc / View / Trigger / Function etc.) has a "Script Run Log" Sproc call at the top to Log the Object name, Version, Who, When, etc. So if you run the script (success or fail) it gets logged. If you (or more likely a Big Rollout Patch Script) runs a bunch of script files, each one gets logged in turn (and as part of rollout we check that QA matches DEV, and then that PRODUCTION matches QA 🙂 )

    So I can find all changes that (that developer) made during the time period of their timesheet entry/entries (for a given Work Ticket)

    The Log only says that the Script was run, not that it was successful. If the code is for an ALTER and there is a syntax error then I will have a LOG record but no change to the object. (i.e. in that instance the Log Record date will be newer than the MODIFY date of the object.)

    We also have a DDL change trigger which logs the DDL change ... so I can also use that to find any actual changes to the object - also whether the ALTER was successful, or not (and also the Before/After code content)

    So I can report on:

    For a given ticket JOIN to all the timesheet entries and JOIN to the Script Run LOG and the DDL Change Log to find all the "events" that occurred.

    This is of course not dependent on me putting a good, accurate, Company Policy Compliant [Newbies may muck up here ...] and truthful 🙂 Commit Message

    The DDL Log also has all the "actual source" of the various modifications that were made. SVN will (for me) only give me the next checkin - for us that only happens after the work has been successfully completed and that will most likely include "other work done today" (but I can see that "Checkin after each Ticket" would be a suitable alternative)

    ... This presents you with a web front end that shows the changed files from those commits in tree view. As you select each file it shows you a diff' on the right. The code reviewer can then highlight individual lines or multiple lines and add a comment, e.g. "select * is bad practice in a query returning data, change it to select column names". You can mark the comment as "needs resolution". After the developer has made the changes and, optional, replied to the comments on the review, the new commits can be added to the review and you can then mark the comment as resolved.

    That's a million times better than my method 🙂 As is happens we already have a very slick DIFF plugin in our DEV web system (usually used to compare CMS Template Content changes ...). It has never occurred to me to hook that up to changes in Sprocs etc. and build THAT into a Code Peer Review system. I'm not sure I even need SVN to get the history, its in my database log backups and the DDL Log. I'll have a look to see if I can hijack that to do my Peer Review from there. I am pipe-dreaming, but perhaps I could also hook that up to our Help Centre too so that I could "improve" the Coding Standard Articles where code peer review finds something sloppy and, shamingly, the coding standards DOCs are equally sloppy!

    FYI - we literally just got a test migration of our projects SVN repo to Git hosted in AzureDevOps done the other day. It took me a while to figure it out and get it right. It takes about 26 hours to migrate nearly 20K commits, because I want to keep the SVN commit history but it worked in the end. If you don't care about your commit history or tags then you can simply just take a snapshot of your latest codebase and be done inside an hour or 2.

    I've been thinking for years that we should move to something more modern than SVN, particularly something "distributed" (if that is the right word) where I could check in my partially debugged code without actually committing it to the tree until I've actually finished that job. That would help me with jumping between my desktop PC and a laptop when out on the road (except I haven't been "out on the road" for a year so that requirement has been moot recently ...)

    But I'm terrified of the change and what I might lose or hidden snags that will then be uncovered. Just the standard change-resistance of old age (and some accumulated wisdom, with age, too I dare say!). There is no way I would give up my change history 🙂 but a one-off migration that took a whole weekend to run would be acceptable. We are a small shop I doubt it would take that long. SVN reports 7,500 commits, 15K files, 500MB

  • @Kristen

    I'd imagine migrating to Git would not take as long since you have got less than half the number of commits.

    Each release we have a single upgrade script for the DB. Which is generated from database object files (SPs, views, UDFs) that changes after a given SVN commit revision and task scripts, i.e. those written explicitly for the task.

    I'd highly recommend having 1 task = 1 piece of work. That 1 task might actually be 1 weeks worth of code. Even if you have an "epic" task that have several tasks in order to break down the work.

    I also find that for newbie team members it is much easier to drill into them you-must-enter-the-task-ID-in-commit-messages than it is anything else. All teams where I work do the same thing.

    Yes, Git is a distribute source control system. So you have the entire repository on your machine. You can work completely offline to the central repo, committing your changes, creating branches and tags and then only when you connected to VPN/online push those commits, branches, tags back to the central repo for other people to have.

    Git does scare me a little to if I'm being honest. It is certainly more complex than using SVN due to it being distributed and more flexible. With some trial migrations and testing using tools (we're going to use SourceTree + TortoiseGit) it can be worked out. Although a lot of the internet will just tell you the commands to do stuff at a command line.

  • peter.row wrote:

    Each release we have a single upgrade script for the DB. Which is generated from database object files (SPs, views, UDFs) that changes after a given SVN commit revision and task scripts, i.e. those written explicitly for the task.

    We have a comprehensive DEV process ... I'm happy to describe it in case it is either of interest to others, or you folk then make any suggestions ... "Why on earth ..." 🙂 would be fine - and, indeed, useful :). But I don't want to hijack the thread unless you are happy.

    I also find that for newbie team members it is much easier to drill into them you-must-enter-the-task-ID-in-commit-messages than it is anything else. All teams where I work do the same thing.

    I hear you, but I would much prefer that the system dynamically deduce and record that. Even our best, most pedantic people who are employed to do formalities data entry ... make data entry mistakes.

    Thanks for your thoughts on GIT. I use Tortoise for SVN and I love that ... dead simple to do the common tasks, and it has been very reliable over the years. If TortoiseGit behaves in a similar way? I am sure I would be very happy with it

  • TortoiseGit is not enough on its own, IMHO. It's only really good for a quick way to get the log of a particular file. For everything else you'd need something else. There are many other options but we've gone with SourceTree, it's free like TortoiseSVN/Git, but supports the complexities of Git much easier than TortGit.

  • The most useful thing I have found in both commit messages and code comments is to document the intent as opposed to what was done ... the intent being far more useful than a random ticket number (and if you have a single ticket number you are insanely lucky ... ticket numbers breed like rabbits making change tracking nigh on impossible without cross-referencing multiple numbers, for which life is too short) ... as a UK native git perfectly describes itself; I stil find myself needing to keep local backups to recover from its random actions at times.

Viewing 11 posts - 16 through 25 (of 25 total)

You must be logged in to reply to this topic. Login to reply