Wednesday, December 21, 2011

Some great software project stats...

35 % of all requirements change in the lifetime of the average software project.

Wednesday, November 2, 2011

Windows Powershell Development (Hell)

I thought MSDN documentation was bad for C# and .NET. Try reading and searching the documentation for Powershell Development. Jesus wept. It is all kinds of awful. Half the links are broken and redirect to "Topic no longer available" as if powershell was 25 years old or something. The font is light grey and barely visible. It would be more appropriate for recipes on a cooking site rather than a technical site. There are huge amounts of text and very few examples and the example they have don't compile. As a final sign of sloppiness the 2.0 SDK downloads into a "/powershell/v1.0" folder, seemingly just for shits and giggles.

Tuesday, November 1, 2011

facepalm - a little mercurial extension

I have been using Mercurial for a few months now and I love it. In particular I love the way it separates the act of committing code from inflicting it on everybody else! It is also extensible and I have written a little python extension called facepalm. This will search the changes you are about to commit for common "gotchas!" such as leaving in mercurial-inserted diff text or my personal weakness - commenting out Ignore unit tests and forgetting to comment back in the Ignore attribute causing the build to fail! D'oh! Facepalm!

Tuesday, October 25, 2011

Everybody owns all the code

On my team I want everybody to own the code collectively. If something is broken and you know how to fix it, then just fix it. Take responsibility for the code as a collective. If somebody fixes your code, say thanks. Don't get protective and defensive. It's not your code - it's the team's code. It's good mantra to have!

Thursday, October 20, 2011

When Agile becomes an excuse

Agile as a term is so overused it has become almost meaningless. Especially after being hijacked by corporate-speak manager types. It has joined the pantheon of Great Meaningless Corporate Buzzwords That Formerly Meant Something in the Tech Community. It has gotten to the stage where I almost physically flinch when I hear it being used in open conversation. I hate what it has become.

Now, as a software engineer I of course recognise the value and revolution agile development and project management practices have brought us over the last 5 - 10 years. But in my experience I have been scarred by its misuse. I have been burned, baby! Many people (I'm one of them) would say if you are doing agile right, it will always be valuable. I just haven't had good experiences in reality. For example, I worked on a team managed by an engineer who professed to believe in the agile manifesto. A few of us knew the truth of course - that this particular engineer was a cowboy-coder and agile was just the excuse needed to ditch every process and standard and start hacking away - "it says it right in the manifesto!".  The sad reality was the delusion that we were now agile was being used to excuse lack of proper consideration for design and poor coding standards. In fact, the main outcome of "being agile" in this team was bad and wildly differently styled code as everybody rushed to get poorly-defined and incorrectly-sized tasks done in two-week sprints. So the customer ultimately suffered. Not to mention the move to scrum happened right in the middle of a 6 month project wasting ridiculous amounts of time in meetings etc. without proper consideration of availability of product owners and decision-makers. The result was developers filling in the gaps in tasks due to the lack of availability and buy-in of product managers. The product bombed with many customers returning it. I remember seeing a comment on a customer forum along the lines of "it looks like it was designed by a bunch of engineers without a clue how we use the product" How right you were sir!

One of the other frustrating outcomes was that this environment allowed cowboy coders to flourish in the team. I think programmers are like people (almost...) in that it is too easy to label a programmer as being a certain type. We all have different characteristics and personalities and of course some key characteristics shine through stronger than others. It is too easy to just label somebody a "cowboy coder" and move on. All programmers probably have some part of them that just wants to check in some code they hacked together but what separates us from the cowboys, is we recognise a professional responsibility of working in a team and recognise that in the long-term that code is going to be of poorer quality and cost more than code that is properly designed, tested and reviewed. I would like to think that coders who don't really care about the code-base and quality and standard of their work, who don't consider proper design and ultimately (let's not sugar-coat it here) don't give a flying fuck about the customer would cease to exist in a modern development team. Unfortunately I have witnessed the opposite! Cowboys can actually flourish under bad leadership and the guise of being agile. Beware!

Another time, I found myself working on a project with no spec or plan (at least none visible to me anyway) and no structure in terms of project management, everything being drip-fed from the ideas in one persons head. I was told this was an agile way to work when I highlighted some of my concerns to my manager. Working like this is demoralizing. I actually dreaded coming to work but it gradually improved and the project was eventually completed. I have tended to move on when I have come across environments like this and there are no shortage of places looking for engineers these days. The problems are that it is difficult to grasp the culture of a place without experiencing it yourself for a few months and of course the grass is always greener as they say...plus you can't move around too much, it looks bad. So the other option is to try to change the culture of a place from the inside, bottom-up. I believe that is one of the hardest things to do in professional life. Another option is to just rant on your blog! It's good to vent.

Tuesday, September 20, 2011

Change of direction and focus

A few blog posts ago I set out a path for attaining an MCTS and blogging about it in the process. I have now scrapped this plan and changed focus. Why? I remembered a post I saw (on G+ I think) about when deciding what to do next you should think backwards from the future - was that a good use of my time? Am I proud of that? I can't honestly say an MCTS answers those questions. So what am I going to do with all this spare time? (Spare time?! yeah right...) 


I have been inspired by an old blog post from Joel Spolsky. It is simply a list of books he recommends (requires) anyone taking the software management course at Fogcreek to read. There are 75 books on the list so reading at the rate of a book every two weeks it will take 3 years to get through all of them. Ambitious though it is, if it took 3 years or even 5 years then what harm? It sounds like a good investment of my time. I'm currently a software engineer and although titles don't matter to me too much, progress and autonomy does. So I see my career going something like senior software engineer (real soon hopefully) and then some sort or form of team lead role. After that who knows? Architect, Product Manager, Software Development Manager? No matter what progress I make and what path I end up on something tells me this will be time well spent. 


Reading (the right) books is one the surest ways to increase your knowledge and with a Safari Books account I can get almost all of these titles for free! All I need now is a decent e-reader. 


I started on the list last night with "The Mythical Man Month" and I'm on Chapter 6 already (reading at night on my Nexus S hurts my eyes so I'm not sure this pace will last). If I got through half the list I'd be impressed!

Friday, September 9, 2011

Returning more than one value from a method.

Reading "More Effective C#" I came across a pretty cool bit (around p.55) about using the tuple type to return more than one value from a method. That's fair enough but the really cool part was the way the author used the using keyword...

For example, in returning a latitude, longitude from a method (you could be returning five or six values etc. with a tuple but lets use two for simplicity) Tuple<int, int> doesn't really tell us too much.
public static Tuple<string, decimal> GetLocation(string address)
{
   int longitude = // get longitude algorithm
   int latitude = // get latitude algorithm
   return new Tuple<int, int>(longitude, latitude);
}

//usage
Tuple<int, int> weather = GetLocation("address");

To give a more meaningful return name you can provide an alias for the return value. 
using GeoLocation = Tuple<int, int>;
public static GeoLocation GetLocation(string address)
{
   int longitude = // get longitude algorithm
   int latitude = // get latitude algorithm
   return new GeoLocation(longitude, latitude);
}

// usage
GeoLocation location = GetLocation("address");

The using keyword continues to surprise me (e.g. as a tidy way of attempting to dispose of something without knowing whether or not it implements IDisposable). Of course if this method was simply better named "GeoLocation" there's be no problem! But it's the concept that's useful to know.

Friday, August 12, 2011

Example of poor web design!

Wow....here's my bank's online banking login page. Notice the separate log-in for people using the Safari browser!! WTF?! Seriously? This is a horrible user experience. It confuses people, especially those who have no idea what Safari is or even what a browser is. If people are confused then they will not use the service. And why Safari? Surely IE6 users should be the ones directed to a separate log-in. Of course, in reality, nobody should be - everybody should use the same log-in and the site should be designed to work across all popular browsers. And surely they could redirect UK customers automatically based on IP? And don't even get me started on the red font...sigh. 




Thursday, August 11, 2011

Good API Design

I'm going through a few iterations of beginning to design an API. I came to a point in writing the operations where I stopped and thought about the parameter (and return types) I was using. The API is an interface for transferring a file. Originally I had two parameters.
void Upload(string fileName, string destinationUri);

I looked at this and thought...mmmmm...too many strings. Maybe I should specify a better type? But then string is nice and flexible right? What is the better approach? I came across a presentation from Joshua Bloch (a Principal Software Engineer from Google) who has a slide about exactly this.













The second and third point are key here
  • "Use most specific possible input parameter type"
Your user will have to construct the more specific type before using the API but as he says, the error then moves to compile time from runtime - this is much better.
  • "Don't use string if a better type exists"
In our case (.NET) there is a better type. The System.Uri type can replace the destinationUri string. In fact, you could argue that it could replace the filePath string too as file:// is a valid Uri.
void Upload(Uri from, Uri to);

That looks okay to me but I think I would prefer a string for the filePath. We don't really want users to pass in any other Uri except for ones beginning with "file://". Besides, the File API in .NET uses "string filePath" in several places so our API would be consistent with what users expect.

So I decided on this (notice I changed the 2nd parameter name to be more descriptive - I think this reads better).
void Upload(string filePath, Uri target);

This will probably change several times again - I like writing API's early and often and getting as much feedback as possible. Once they are released you can never change them only add to them! Or piss off a lot of your users by breaking backwards compatability! I'm looking at you Python.

Tuesday, May 17, 2011

MVC is everywhere now

I was recently talking with another developer about ASP.NET MVC and it struck me how many modern frameworks and development environments are set-up for MVC or some flavor of it. I have come across it now in iOS and Android development, in Django in a little side-project I am doing and in WPF and Silverlight as MVVM. Here is a list of different technologies that I have come across using some flavor of MVC.
  • ASP.NET MVC
  • WPF
  • Silverlight
  • Ruby on Rails
  • Django (Python)
  • iOS
  • Android
  • Java SE with MVC
In a recent WinForms project I used yet another flavor called Model-View-Presenter where the Views and Model do not talk at all. An IView and an IModel is injected into the Presenter through the constructor. It is called Presenter-First and encourages dumb views and TDD. You can easily mock your views and models and test the Presenter - which is where all the behavior ends up. Really cool...


After looking it up, MVC seems to have originated as a pattern decades ago in the Smalltalk community. It went out-of-fashion for a time but I think it is safe to say it is definitely back in vogue now. What I find interesting is that a lot of these new frameworks "force" you into using MVC insofar as you have to work very hard to not use the pattern. This is called "Convention over Configuration" - why allow the developer to configure the framework and choose different architectural patterns? If the pattern is set for you, this aid rapid development and unit testing.

One thing is for sure - you NEED to know this pattern. It is almost impossible to avoid but the good thing is that it is very easy to learn...

Tuesday, May 3, 2011

Changing Registry Settings

I recently wanted to change a registry setting programmatically. This is really straight forward actually...
var key = Registry.CurrentUser.OpenSubKey(
@"Software\Microsoft\Windows\CurrentVersion\Explorer\Advanced");
key.SetValue("Hidden", 1);

Ah ha! Not quite as easy as I thought - this resulted in an exception

System.UnauthorizedAccessException was unhandled. Message="Cannot write to the registry key."

Luckily there is an overload for OpenSubKey that takes a boolean parameter called writable. So the following code works like a charm.
var key = Registry.CurrentUser.OpenSubKey(
@"Software\Microsoft\Windows\CurrentVersion\Explorer\Advanced",
true);
key.SetValue("Hidden", 1);

Thursday, April 28, 2011

Cross Thread calls on WinForms

In a multithreaded WinForm app, one of the most common exceptions you'll see is in trying to access a form, or controls on a form, from a different thread than the one it was created on. Fortunately there is an easy way to check if a control is being accessed from the thread it was created on - the InvokeRequired property.

This simply checks if the handle was created on the same thread as the caller. If this Property is set to true then you need to marshal the call to the UI across the thread. There are methods provided for this too - use Invoke and BeginInvoke to correctly marshal the call to the thread that created the Control. From MSDN, Control.Invoke "Executes the specified delegate on the thread that owns the control's underlying window handle".

Most of the code snippets you see on the web for this are out-of-date. Although they work well there are better solutions using built-in features of .NET that results in less code (woo-hoo!).

The old way is to declare a custom delegate
public delegate void UpdateFormTextChanged(string message);

Then you will see that delegate being invoked from the form. If parameters are to be passed they are done in an object array (yuck).
   
public void UpdateProgressMessage(string message)
{
if (!InvokeRequired)
{
DownloadStatusTextBox.Text = message;
}
else
{
UpdateFormTextChanged updateText = SetProductName;
Invoke(updateText, new object[] { productName });
}
}

A simpler way of doing this without the need to create custom delegates is to use Action<T> which takes one parameter and returns void. You replace T with your parameter type making it strongly typed - no objects or object arrays.
public void UpdateProgressMessage(string message)
{
if (!InvokeRequired)
{
DownloadStatusTextBox.Text = message;
}
else
{
Invoke(new Action<string>(UpdateProgressMessage), message);
}
}

For a method with no parameter and void return type you can use the Action() method (in .NET 3.5 and higher) or MethodInvoker in .NET 2.0
   
public void ShowProgressForm()
{
if (!InvokeRequired)
{
ShowDialog();
BringToFront();
}
else
{
// .net 2.0
Invoke(new MethodInvoker(ShowProgressForm));
// .net 3.5 +
Invoke(new Action(ShowProgressForm));
}
}

For more than one parameter you can use one of the many overloads of Action<T>. But wait! I don't see any available in .NET 2.0. Yeah, .NET 2.0 includes Action<T> but not Action<T1, T2> or Func<T, TResult> etc. but it does support Generics of course so you can write your own delegates to mimic these 3.5 features. For example, if you want to call a method on a form that has two parameters - for example, updating a progress bar
public delegate void Action<T1, T2>(T1 arg1, T2 arg2);
public void UpdateProgress(int total, int downloaded)
{
if (!InvokeRequired)
{
ProgressBar.Maximum = total;
ProgressBar.Value = downloaded;
}
else
{
Invoke(new Action<int, int>(UpdateProgress), new object[] {total, downloaded});
}
}

Wednesday, April 27, 2011

Swallowing Exceptions (Gulp...)

I do not agree with the mantra that you should NEVER swallow exceptions. I know some people who will fight you on the spot for suggesting otherwise but like any rule there are always exceptions. Don't get me wrong, I think it is a great rule-of-thumb and should be followed most of the time but I've come across a case recently where I swallow exceptions and I don't feel guilty about it...

  • Logging throws an exception

Sometimes you don't care if logging fails. Sometimes logging is really not that important and you absolutely do not want to interrupt the application (and the Customer) just because logging has failed. Right?
try
{
Logger.Write(message);
}
catch
{
}

So there is a real-life scenarios where I think it is okay to swallow an exception...Here is a good discussion on the topic on StackOverflow

Comparing Anonymous Types

If you have multiple anonymous types that have the same Properties, with the same type and in the same order then the compiler treats them as the same type.

For example, if I have two anonymous types declared as follows

var a = new { Name = "Peter" };
var b = new { Name = "Peter" };

The compiler sees these as the same type (the Properties are both called Name, they are both of type string and all the Properties are in the same order). Because Anonymous Types inherit from System.Object you can check if these two instances are equal using Equals(). Anonymous Type instances will be equal if their properties are equal. So the following outputs "True" because the Name properties are equal (strings that match exactly).

Console.WriteLine(a.Equals(b));

However, if I changed the case of one the Names to "peter", this would result in "False" because the two properties are no longer equal - String overrides Equals and ignores case in the checking for equality. It is also worth noting that using operators when checking for equality will not give you the result you might be expecting. You cannot have any members in an Anonymous Type other than Read-Only Properties meaning you cannot override a method from System.Object including operators. So the following code results in "False".

var a = new { IsAnon = true };
var b = new { IsAnon = true };
Console.WriteLine(a == b);

Thursday, April 21, 2011

Changing Start Mode of a Windows Service

Recently I needed to change the Start Type of a Windows Service from another application. Some PCs were infected with the Conficker worm. One of the things this worm does is shut down some Windows features that might help in discovering/removing the worm. One of the services it affects is BITS - used in downloading Windows Updates - by stopping the service and changing it's start mode to Disabled.

There is a managed option in .NET - the System.ServiceProcess.ServiceController class which exposes some useful Properties and Methods for managing a Service. However, it provides no way to change the start mode of a Service. In looking into this I came across this question on StackOverflow which suggests the best way to achieve this is through the Win32 API using P/Invoke.

The awesome wiki pinvoke.net is a great resource to get started. It has helped me many times in the past and it certainly helped me to write some of this code here.

First of all we need to make calls to functions in the advapi.dll. We need to call ChangeServiceConfig, OpenSCManager and OpenService.

ChangeServiceConfig is where we can actually make the change we want to the Start Mode. But you need to pass this a handle to a service. So we also need to call OpenService. This in turn requires a handle to a SCManager so we need to call
OpenSCManager. Here is the code - feel free to use and improve. It is a static class that provides one method ChangeStartMode

public static class ServiceHelper
{
    [DllImport("advapi32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
    public static extern Boolean ChangeServiceConfig(
        IntPtr hService,
        UInt32 nServiceType,
        UInt32 nStartType,
        UInt32 nErrorControl,
        String lpBinaryPathName,
        String lpLoadOrderGroup,
        IntPtr lpdwTagId,
        [In] char[] lpDependencies,
        String lpServiceStartName,
        String lpPassword,
        String lpDisplayName);

    [DllImport("advapi32.dll", SetLastError = true, CharSet = CharSet.Auto)]
    static extern IntPtr OpenService(
        IntPtr hSCManager, string lpServiceName, uint dwDesiredAccess);

    [DllImport("advapi32.dll", EntryPoint = "OpenSCManagerW", ExactSpelling = true, CharSet = CharSet.Unicode, SetLastError = true)]
    public static extern IntPtr OpenSCManager(
        string machineName, string databaseName, uint dwAccess);

    [DllImport("advapi32.dll", EntryPoint = "CloseServiceHandle")]
    public static extern int CloseServiceHandle(IntPtr hSCObject);

    private const uint SERVICE_NO_CHANGE = 0xFFFFFFFF;
    private const uint SERVICE_QUERY_CONFIG = 0x00000001;
    private const uint SERVICE_CHANGE_CONFIG = 0x00000002;
    private const uint SC_MANAGER_ALL_ACCESS = 0x000F003F;

    public static void ChangeStartMode(ServiceController svc, ServiceStartMode mode)
    {
        var scManagerHandle = OpenSCManager(null, null, SC_MANAGER_ALL_ACCESS);
        if (scManagerHandle == IntPtr.Zero)
        {
            throw new ExternalException("Open Service Manager Error");
        }

        var serviceHandle = OpenService(
            scManagerHandle,
            svc.ServiceName,
            SERVICE_QUERY_CONFIG | SERVICE_CHANGE_CONFIG);

        if (serviceHandle == IntPtr.Zero)
        {
            throw new ExternalException("Open Service Error");
        }

        var result = ChangeServiceConfig(
            serviceHandle,
            SERVICE_NO_CHANGE,
            (uint)mode,
            SERVICE_NO_CHANGE,
            null,
            null,
            IntPtr.Zero,
            null,
            null,
            null,
            null);

       if (result == false)
       {
           int nError = Marshal.GetLastWin32Error();
           var win32Exception = new Win32Exception(nError);
           throw new ExternalException("Could not change service start type: "
               + win32Exception.Message);
       }

       CloseServiceHandle(serviceHandle);
       CloseServiceHandle(scManagerHandle);
   }

To use this just provide an instance of ServiceController and a ServiceStartMode.

var svc = new ServiceController("BITS");
ServiceHelper.ChangeStartMode(svc, ServiceStartMode.Automatic);

// You can then Start the service if necessary. 
if (svc.Status != ServiceControllerStatus.Running)
{
   svc.Start();
}

// And of course you should close the service when no longer needed
svc.Close();
http://msdn.microsoft.com/en-us/library/system.serviceprocess.servicecontroller.aspx

Wednesday, April 13, 2011

I love moq

Moq (pronounced "Mock-you" or just "Mock") is the only mocking library for .NET developed from scratch to take full advantage of .NET 3.5 (i.e. Linq expression trees) and C# 3.0 features (i.e. lambda expressions) that make it the most productive, type-safe and refactoring-friendly mocking library available.

Mocking frameworks allow you to mock out interfaces and then wield magical powers over them by having methods return whatever values you want, start firing events and even raise exceptions. All so you can focus on testing your code without the pain of setting up the implementation of the Interface to do all the various things it needs to do for you to properly exercise your code.

Set up a Property to return a certain value
_mockModel.Setup(m => m.Description).Returns("Description");

Fire an event from the mock with certain event args
_mockModel.Raise(m => m.DownloadStarted += null, EventArgs.Empty);

Raise an exception with certain arguments
_mockModel.Setup(m => m.ResumePendingDownload()).Throws(new InvalidOperationException("error"));

Verify (like NUnit's Assert) that a method was called on the mock with certain parameters
_mockView.Verify(v => v.ShowDownloadProgressView("Progress"));

Slightly more advanced setups can also be achieved with ridiculous ease.

Verify that a method was called on the mock but you don't know the exact parameters at design-time? You can use the It.IsAny<T> feature when you know the type expected.

_mockView.Verify(v => v.ShowDownloadProgressView(It.IsAny<string>());

Verify how many times a method is called
_mockView.Verify(v => v.ShowDownloadProgressView(It.IsAny<string>()), Times.Exactly(1));

Pretty cool...

Wednesday, April 6, 2011

Bug fixing TDD-style

So some code I wrote resulted in a bug. This is a natural part of software development of course. I'm reminded of that famous programming quote

"If debugging is the process of removing software bugs, then programming must be the process of putting them in."

The standard suggests we introduce a bug every 10 lines of code we write and I knew a programmer who could actually introduce a bug without writing anything ;)

So, how will I address this bug? Well, here is an oh-so-simple process that I follow.

1. Write a failing Unit Test that proves the bug
2. Fix the bug
3. Watch the test pass and smile

By writing the failing unit test first that recreates the bug, you are improving the unit test suite. You are also preventing that bug from happening again - you are closing the loop on it.


Wednesday, March 30, 2011

For anyone wanting to get LAMP set-up on their linux distro I came across a cool and quick command. If you do not need to make changes to configuration then this way will do fine.

# apt-get install tasksel
# tasksel install lamp-server

Simple as that!

Friday, March 25, 2011

Required use of the "this" keyword in C#

When it comes to member variables I prefer an underscore and I'm not going to apologize to the Hungarian notation police! It fits my eye and I like the way it looks in my code. However, the team that I currently work in has different ideas and the standard is to use "this" everywhere...

Now I'm okay with this obviously. Whatever the team chooses is fine by me and once everybody agrees to use the same standards then that is the important thing.

It is interesting that Resharper highlights the redundancy of it by default Qualifier 'this.' is redundant and Stylecop warns you to include it! I find it litters up your code pretty quickly with noise.

Anyhow, I really wanted to find out the uses of the keyword that are not related to style. So here is a list of required uses (i.e. you absolutely have to use it and it is not just a style choice)
  1. To qualify members hidden by the same name
  2. To have an object pass itself as a parameter to other methods
  3. To have an object return itself from a method
  4. To declare indexers
  5. To declare extension methods
  6. To pass parameters between constructors
  7. To internally reassign value type (struct) value.
In these cases the 'this' keyword has a clear purpose, in all other cases it is a style issue. Of course the key thing here is that it really doesn't matter. The compiled code has the IL equivalent of this inserted anyway whether or not you include it in your code and there are definitely more important things to worry about!

Some interesting links on the topic









Thursday, February 24, 2011

Drive-by Code Review

I decided to write a blog post about code review. It is something I get very passionate about when I see it being done incorrectly and it frustrates me that it has become a tick-the-box exercise for some.

I call it Drive-By Code Review. This is where you grab the guy next to you for "a quick two-minute code review" and nothing constructive happens...you just get to check your code in with a nice comment that it was reviewed by X. What is the point in that?!

Of course this can go to the other extreme too. Formal reviews with groups of people that take hours where every line of code is scrutinized to death...normally this is too much but of course has its place in places where the smallest mistake could be fatal (literally...)

I have read some interesting articles on code review recently and I would like to offer my cut-down lightweight process that I would like to see implemented where I work. I think these steps are generic and can be applied to any team but we are a .NET team that uses a CI server along with StyleCop and FXCop so I have mentioned these as part of my process.

1. Explain context of the change I am reviewing.

Every change should be tracked to a bug or feature or ticket or story or whatever is the driver for this change.

2. Show me your unit tests.

This obviously allows me to see that you have actually written unit tests but it really allows me to see your code from the consumer of the code's perspective.

3. Show me your code.

Your code should do what it needs to and no more. It should follow the coding standards of the team.

4. Show me zero StyleCop warnings and zero FXCop violations.

No broken windows here - every warning or violation of our rules should be treated as an error and fixed.

5. Show me all this building on the Continuous Integration server.

Our builds should fail if there are any violations of our rules or any unit test fails.

I believe if our code passes these tests and gets this far then it is in pretty good shape to be handed over to our QA team for system testing, integration testing etc.

One other important point I feel is worth mentioning is that if your change impacts on any other component or library etc. then somebody who has knowledge in that area should be part of the code review. This just makes sense to me.