SourceTree versus Gitk

Git is a command line focused tool with hundreds of options. You can even visually see a graph of commit history in the terminal using git log with –graph. The default visual git tool is gitk which makes a reasonable GUI client for digging through history or doing ad-hoc code reviews.

A few years ago someone suggested looking at Atlasssian’s SourceTree as a better option. I’ve used Atlassian tools in the past and generally found them to be well designed a useful. As it’s a free tool it was an easy option.

SourceTree has been marketed in many ways as a visual tool for users new to git. I think it’s probably a bit dangerous to use in this way. You’re better off learning the simple workflow through something like Try Git and sticking to the command line. SourceTree can be a a great supplement showing you in a visual way exactly what’s going on.

I use SourceTree in an entirely read-only mode. It’s easy to see the various branches and merges and walk through history. Since we currently don’t use Gitlab or a similar tool for git I use it for ad-hoc code reviews. It’s easy enough to step through commits fairly quickly. When I have comments I simply screen capture the view from SourceTree and attach it to a simple email. It’s primitive, but effective enough. Other than that my most common use case is perusing stashes as I sometimes let things stack up there and I can quickly bring them up to see if there’s anything worth keeping.

After three years it still serves my purpose much better than gitk and I find the layout more efficient and powerful.

Developer Expectations

I came across a note of mine from last year on my baseline expectations for developers:

  • All code is checked into source control on an hourly basis or at most daily.
  • Every project has an automated build. (Maven, Ant)
  • All projects are setup in continuous integration (Hudson)
  • All code follows the current Java/Groovy coding standards.
  • Unit test coverage of new code must meet a 70% target. TDD is preferred.
  • Code reviews or regular pair programming are required.
  • Code should meet a standard of low cyclomatic complexity through refactoring and design.
  • Some level of functional, integration, and acceptance tests should be performed.
  • High value documentation is maintained.

Testing .NET Code Behind

Running through a recent code review on an outsourced internal project I came across a new issue. The developers have built a few SharePoint Web Parts in using ASP.NET. I gently asked where the unit tests where since I’d dug around in source code and not seen any.

The developers looked a little surprised and sort of thought I was talking about manual unit testing, but I reiterated that I meant NUnit or MSTest unit tests. There was a short discussion. What followed was they couldn’t see how they could unit test the Page Behind code. This is an unfortunate old story.

Testing controllers in most web frameworks is painful. The classic example is Struts Action classes where you can use StrutsTestCase, but the solution isn’t elegant. JSF was an even worse experience, especially the 1.0 spec. Lately messing around with Grails has made it a bit easier, but I still find it to test the controllers through a front end functional test like Selenium. Of course in Rails it’s fairly trivial.

I’ve followed some agile bloggers in the MS world for some time and I knew this problem existed in the ASP.NET world. I knew a lot of them advocated a model-view-presenter (MVP) design to allow for easy unit testing. I assumed with Microsoft MVC out now that this problem is solved. It appears to be a much better story, but currently Microsoft MVC doesn’t integrate directly with Sharepoint.

The next step was to find a specific library. It turns out there is one much like StrutsTestCase or other libraries that make it easier to test web controllers. NUnitAsp to the rescue, coded by no less than James Shore. Then I read the note on the front page from about a year ago:

NUnitAsp still has some dramatic flaws: no support for Javascript, tests running in a different process than ASP.NET, difficulty setting up sessions. Most people ended up using it for acceptance testing, rather than unit testing, and Selenium, Watir, and the like are better for that. Most folks “in the know” are using presentation layers to make ASP.NET so thin that a tool like NUnitAsp isn’t helpful.

James Shore’s note on NUnitASP Development

So much for a silver bullet. Ever the optimist I had hoped for something like this:

  • Quickly find a nice explanatory tutorial and send the developers the link.
  • Answer a few questions as they experimented with their first tests.
  • Hook up the project to a continuous integration server and start running the tests.
  • Next code review we talk about the adoption of an MVP approach.

Looks like this is going to require some more work.

Bridging Development Realities

Denis over at One Brike At a Time recently expressed frustration at having to use low trust practices:

It used to be that I did not care much for defensive programming outside of a system’s boundaries. It used to be that I did not really care or believed in strict source control (as in authorise only some people to touch a particular file). It used to be that I did not really believe in code reviews.

I now care for these and it is not a good thing. I care about these things because I do not trust the developers I work with to do the right thing, whether it is making the code better, improving their style, teaching me new things, or even write decent unit tests.

The frustrations are real. Teams are often at many different levels of skill and experience. Even minimal unit testing is not a common practice in many shops. You can still find developers who look surprised when you ask where their code is checked in.

Lower trust practices can be a bridge:

  • Source control is a given. Developers who check in bad code you might force you to lock things down for now. Continuous integration is a way to grow out of this. If all of your projects are being built with any change, you can quickly discover if someone’s injected some issues. This might mean setting up notifications for even successful builds for non-active projects.
  • Code reviews don’t have to be a negative. I’ve used them and still use them more for mentoring than as an audit of bad code. Code reviews are easy to get wrong, but used safely they can improve the code base and improve your coders through sharing. And I always leave the easy out–no code reviews if you’re pair programming.

When To Code Review Changes Versus the Whole File

magnifying_glass

Reading through some old documentation today written up a few years ago by our Agile coach I came across a suggested rule of thumb for code reviews:

  • If more than X lines of code changed in a file then do a full code review of the file.
  • If less than X lines of code changed you can just review the diffs.
  • Developers should decide on where to set the bar for lines of code.

Strikes me as a pretty reasonable option. We’ve been reviewing whole classes except in a few circumstances, but we never set a rule on how many lines of code make it worth doing a full review. Maybe 20 lines is a good number.