Showing posts with label workflow. Show all posts
Showing posts with label workflow. Show all posts

Wednesday, 9 December 2009

The #1 thing to learn if you're new to git

Now I'm not 'new new' to git anymore, I've been using it off and on for at least a couple of years, but I've been using cvs/svn for many more years than that. git has a steep learning curve[1], and have found the git-svn crash course invaluable. But there was one thing missing:

The #1 thing I wish I had learned when I was new to git:

...is that you must commit to git *far* more frequently than you do to svn.

From my thrashing about, I have discovered there are many more 'dangerous' commands in git than in svn, and it's really easy to get yourself into a 'stuck'[2] state.

git actually provides a whole set of tools that will help you get back out of whatever hole you've dug yourself into... but you're likely to end up having lost your working-copy changes[3] along the way. So the best practice now is to commit often - so that everything is in the repository: there is never a big working copy to lose.

I had a lot of trouble with that as it considerably changes the workflow I'd built up over the last decade or so. From cvs to svn and then adding agile on top, my workflow is now roughly:

  1. Checkout a fresh, clean copy of the repository (or svn up to achieve same effect)
  2. add your tests and make some changes
  3. check the rest of the tests still run - make changes until they do
  4. do an svn st to see if there are any files I've forgotten to svn add - add them
  5. do an svn diff and see all my changes - eyeball them to make sure there's nothing obviously wrong
  6. do an svn up to pull in latest changes by others and make sure the tests *still* pass
  7. svn commit

So at all times, everything would be *not* checked in - all of it just sitting in the local working copy until I was sure it was ready.

The problem is that, on the surface, git appears to support this workflow perfectly. All the svn-commands described above have git-equivalents (they're even called the same thing) and so you can (supposedly) transition smoothly over to git with only minimal effort. Even adding a branch, rather than hacking on master is not too far a departure from svn-style workflow, as branching is familiar in svn, and git just gives you a beezier easier interface.

So where does it break down?

Well, in my case, usually at the second-last point. A git-pull can completely mess you up if you get to a merge-conflicted state. You can't commit your working-copy because of the merged state, you often can't even properly diff because you've got a mush of the git pull's changes plus your own changes and no way to tell which is which. and there seems to be no obvious way to 'just commit the merge-conflict changes' or update the files that are conflicted and just *tell* git that they're not conflicted anymore... the way you can in svn. So at this point you're screwed.

What makes it worse is that at this point you often don't know exactly what commands you did to get you here - if you're anything like me, you've probably tried a whole bunch of stuff only partly understanding exactly what it does. Each command simply tells you in it's own way that you can't do that. You can look up what you're supposed to do to fix it - but generally find that's just another command that tells you that you can't do it either. So you feel like a truck that's stuck sideways in a narrow alley and can't even understand how it got here, let alone how to get itself back out.

Frustrating!

Underlying that is the, quite reasonable, fear that you may lose all your work[3] since your last commit...

and of course that's because we're used to the underlying 'don't commit until' mentality that we may not even be aware we are sporting.

don't commit until (perfect)

The workflow I described above is a perfect example of this mentality. It makes sense to hold back on committing anything until it all works. After all, you know that the moment you commit, the CI server will pull all your changes and let everybody on the team know that you just broke the build (again). So eventually you adopt a "don't commit until the tests pass" workflow, and keep everything in your working copy until everything's green before committing to the svn repository. Fostering this "don't commit until it's right" mentality is a natural consequence of not wanting to look like an idiot to your colleagues, and works in perfectly fine with the svn-based workflow.

but git doesn't work that way!

Or should I say that git doesn't *need* to work that way. After all, you still need to make sure that your tests pass and don't break on the CI server... but what I've found is that you need to get over the whole git commit thing. It may be named the same thing as svn commit - but it doesn't mean the same consequences (eg that your colleagues will all see that the feature's only half-complete and the tests are all spazzing out).

Instead, change the way you think: the command that *matters* now is actually git push. You can commit whatever you like to your local repository, even if it breaks the build; it's only when you push up to the remote repo that it must be working perfectly.

Any other problems with this?

Unfortunately, there are some other consequences to this small change in workflow. One of them being the fact that you can't do a 'git diff' that covers all your changes since last push. git status and git diff are *just* like svn status and svn diff - they check against the latest commit, not the latest 'push to remote', which means it's hard to do a complete check of all your changes before going 'live'... you have to just trust that all your smaller commits all add up to the Right Thing.

That makes me feel uncomfortable as I like to be sure. I know about human error - and I know that I'm as prone to it as the next guy...

Having to make a patch-against master and then read through *that* (which is far less clear to read than a diff) is not a good substitute, IMO. If anybody has a good way on how to mutate the workflow to accommodate this I'd love to know.

a new workflow?

I'm still working on this but so far I've got:

  1. Clone a fresh, clean copy of the repository (if I don't already have one)
  2. git checkout -b my_new_branch
  3. add tests and/or make some changes
  4. do a git diff and check this change - eyeball it to make sure there's nothing obviously wrong
  5. git commit (then repeatedly return to step 3 until 'done')
  6. check the rest of the tests run - make changes (and git commit) until 'done done'
  7. do a git pull origin master to pull in latest changes by others and make sure the tests *still* pass
  8. fix any merge-conflicts and commit the merge
  9. git push

This is still a work-in-progress, and I would appreciate informed opinions, advice or your own war stories.

Notes:
[1] IMO the git developers could learn a thing or two from Kathy Sierra... but that's another topic.[4]
[2] If you've ever got into a state where you can't run git commit because you're in a 'failed merge', you can't git pull because you get 'fatal: merging of trees' or 'Automatic merge failed; fix conflicts and then commit the result.'. You edit the files to un-conflict them and try to reapply your stash you suddenly get 'CONFLICT (content): Merge conflict in ...' again... After thrashing around for a while between git stash, git pull, updating merged files then trying to re-apply your stash before git committing... I can tell you where I wanted to stick git.
[3] If you're anything like me, you look on the words git reset --hard HEAD with some trepidation. You just can't quite believe that blowing everything away in your working copy is the only way out of a simple merge-conflict.
[4]...and please don't just tell me that git is open-source and I should just go hack on git myself if I hate it so much. In theory I absolutely agree with you, but in practice I can only work on one thing at a time - and right now I'm still working on Active Resource, some projects of my own, a novel...

Friday, 9 May 2008

Lay a Wizard over your controllers

Adding a wizard interface to a pre-existing system

Our system requires a user to enter in a large data set and several preferences to complete their order. It all works ok, but new users can get confused when presented with a busy and convoluted screen full of scaffolds and forms - which the initial (advanced) interface currently looks like. So we decided to add a step-by-step "create your order" wizard to help new users get an idea of what they need to do in order to create, edit and submit their orders.

As it happens - the ordering (and number) of steps in the middle don't matter too much. It's all just entering random bits of information. As long as the user creates the order at the beginning and submits it at the end - it doesn't matter if they add their widgets first, or set up their preferences first. In fact, given we don't care about the specifics of the steps it makes some sense to make sure this wizard is highly flexible.

Lucky for us - Ruby gives us that luxury.

Now, we already know the system does all of the existing parts of the process - it's currently happily creating/updating and submitting orders as we speak. So "all we need" is a new overlay of views and a new path through the controllers.

But how to do that in a nicely independant way?

First - make thy controller

We'll need one of those, and we'll need an action for each step in the process - plus an index action thrown in for good measure. So lets start with:

script/generate  controller WizardController create_order add_widgets specify_preferences review_and_submit index

Adding an action per step lets us make nice, named routes that look good in the URL-bar for our user. They also make routing easy. We only need to add the route:

map.wizard '/orders/:id/wizard/:action', :controller => 'wizard', :id => nil

Note that this is great for all actions on an existing order thats saved in the db and has its own id. However, a new order won't have an id yet, so we'll need a route to cover that too:

map.new_wizard '/orders/new/wizard/', :controller => 'wizard', :action => 'index'

Here's the basic wizard controller. Note that we try to fetch out the order before each step with a common before_filter. The "index" action will dump the user into the second wizard-step if they specify an existing order id. This is so they can come back to the interface from, say, a list of orders and not get dropped (confusingly) on the "new order" page.

# controls the "new order wizard" functionality
class WizardController < ApplicationController
  # this list holds the current ordering of the wizard steps. It's used by the 
  # workflow-display code and anything that needs to know what the "next step" is
  WIZARD_STEPS = [:create_order, :add_widgets, :specify_preferences, :review_and_submit]

  before_filter :login_required
  before_filter :prep_for_step

  # the various steps in the process of the wizard
  # Note - they are named, rather than numbered so that they can be
  # adjusted/renamed/removed without worrying about their ordering.
  # There's no code as most of them all operate simply off the order 
  # (which is fetched out for every step) and have a same-name template
  def create_order
  end
  def add_widgets
  end
  def specify_preferences
  end
  def review_and_submit
  end
  def index
    # if we've found an existing order, go straight to the first data-entering step
    return render(:action => WIZARD_STEPS[1]) if @order
    render :action => WIZARD_STEPS.first # otherwise begin at the beginning
  end

  private ############################################################
  # preload the order, if an id has been passed  in - or create a temporary one if not.
  def prep_for_step
    return false unless logged_in? # to be sure, to be sure
    @order = current_user.orders.find(params[:id]) unless params[:id].blank?
    @order ||= Order.new(:user => current_user)
    @wizard_step = params[:wizard_step] # save current step to pass into templates
    true
  end
end

Make a template for each step

Each action will need an associated template showing the options available to a user for that step of the process. This template will probably import partials from your pre-existing views. After all, you're laying this interface over an existing one - so template re-use is a Good Thing. The view re-use gives similar operations a common appearance across both the wizard and advanced interfaces. This helps a user make the step from the simple to the complex interface, due to a pre-existing familiarity with the appearance.

So why not just use the same views as your existing system? There are a few benefits to having separate wizard templates.

First is to provide a unifying, common "wizard interface". Eg a visualisation showing where the user is in the process workflow. Possibly ven a subtle change in the page style to make sure a user knows when they are "in the wizard" as opposed to the rest of the site. Clues as to orientation are surprisingly helpful, especially to new users who are unfamilliar with the layout and content of your site. Every bit helps.

Second - this interface should contain extra instructions on the requirements of the current step. You know for a fact that the wizard will be used by the novice users of your system - so make sure you support their needs by providing extra help. The purpose of this wizard is to hand-hold your newbies through a difficult and complex procedure. Now that you've cut it up into bite-sized pieces, make sure you don't let them down by failing to explain each step.

Finally, the wizard interface should be much simpler than the normal view templates. As aleady stated, these users will be brand-new to your site. They don't want the advanced power-user features just yet - they just want to know how to get started. Any highly complex extras should be left out. You can keep the advanced uber-functions for when you user is confident enough to move beyond the wizard to your standard (now advanced) interface. Just keep the bare minimum that a user will need for this wizard to be functional, without crippling it so badly as to be useless. ;)

Displaying the workflow

Your user will need to know: a) what steps are in the wizard b) what step they are currently on. This is basically just a fancy tab-navigation structure, with the current one highlighted. I've covered this sort of thing in my tabbed navigation articles, so I won't repeat the details here.

You will need to store the set of steps somewhere accessible. In this case, we figured it made most sense to store the list as a constant in the WizardController itself. I also recommend using little images of right-facing arrows in between each "tab" to give the perception of flow.

I generally add a step-specific overview (eg "In this step, add widgets to your order") to get the user oriented. For us this goes directly underneath this tab-list so there's an obvious visual connection between the overview and the current-step. Don't make it long or it won't get read. Brevity gets the point across better!

Interfacing with the existing controllers

So, we have nifty templates that display what a user should be updating next - eg a list of possible widgets for the user to add to their order. So when they click on the "add widget" button - what happens next?

We already have an "add widget" action in our WidgetsController and we want to re-use that because duplicating controller functionality is just a nasty maintenance nightmare waiting to happen. So, the forms that are displayed on the templates need to point at the usual controller actions just like your original views. The problem is that when the WidgetController's "create" action is done, it's likely to send the user on to "order_widgets_path" (just like normal) - when we really want it to come back to the current step in the wizard.

So - how does the WidgetController know we want the wizard instead of the usual control path? and if so - how do we know what step of the wizard we need to pass on to? and finally, can we do all this without becoming terribly hard-coded and brittle?

Firstly, we can tell the other controller that we are coming from the wizard by passing the current wizard step in a variable in the required forms/links thus:

  <% button_opts = {:order_id => @order.id, :escape => false, 
                    'wizard_step' => 'add_widgets' } -%>
  <%= button_to('Add to order', new_order_widget_path(button_opts.merge(:id => widget.id))) -%>

This will probably require some hacking on any existing partials that have embedded forms to add a field-repeater eg:

  <%= hidden_field_tag('wizard_step', @wizard_step) if @wizard_step -%>

Now you can add a check in the relevant controller actions that tests if this step is present. If so, we need to go back to the given wizard step... but checking specifically for param[:wizard_step] is a bit nasty. What happens if we have to change how we specify that we're currently executing a wizard-step? So pull it into a common method that does a "test and redirect" thus:

In application.rb:

  # redirects the user on to the given step in the wizard process
  def redirect_to_wizard_step(the_id, step)
    return false unless current_wizard_step # allow controller to continue on as normal
    redirect_to wizard_path(:id => the_id, :action => current_wizard_step)
    return true
  end

  # Redisplays the current step in the wizard process.
  # Note: only use this if the current action was *not* successful.
  def redisplay_current_wizard_step(the_id)
    return false unless current_wizard_step # allow controller to continue on as normal
    # render template is necessary as we are likely not coming via the
    # wizard controller
    render :template => "wizard/#{current_wizard_step}"
    return true
  end

  def current_wizard_step
    params[:wizard_step]
  end

In WidgetController:

  def create
    @widget = Widget.new(params[:widget])
    @widget.order_id = params[:order_id]
    # to re-render wizard-step in view - if present
    @wizard_step = current_wizard_step 
    respond_to do |format|
      if @widget.save
        format.html { 
         # if we came here through the wizard - move on via the wizard process
          return if redirect_to_current_wizard_step(@widget.order.id)
          redirect_to order_path(@widget.order) }
        format.xml  { head :created, :location => order_widget_path(@widget.order, @widget) }
      else
        format.html { 
         return if redisplay_current_wizard_step(@widget.order.id)
          render :action => "new" }
        format.xml  { render :xml => @widget.errors.to_xml }
      end
    end
  end

That's about it - happy wizarding.